Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper typing for run_sync method is now possible #197

Open
michaeloliverx opened this issue Jan 9, 2022 · 4 comments
Open

Proper typing for run_sync method is now possible #197

michaeloliverx opened this issue Jan 9, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@michaeloliverx
Copy link

Is your feature request related to a problem? Please describe.

I authored sqlalchemy/sqlalchemy#5777 which added a typevar to run_sync to infer the return value. At that time is was not possible to infer *args or **kwargs of the callable passed and nor was it possible to infer that the first argument of type Session passed automatically for you. Since Python 3.10 has been released we have additional typing constructs typing.ParamSpec and typing.Concatenate, both of which are available on older python versions using the typing_extensions backport.

Describe the solution you'd like

Properly type all of the run_sync methods, something along the lines of the following should do the trick (I haven't set up an environment to test):

if sys.version_info >= (3, 10):
    from typing import Concatenate
    from typing import ParamSpec
else:
    from typing_extensions import Concatenate
    from typing_extensions import ParamSpec

_T = TypeVar("_T")
_P = ParamSpec("_P")

class AsyncSession(
    _AsyncSessionTypingCommon,
    _SessionInTransactionTypingCommon,
):
    ...
    async def run_sync(
        self,
        fn: Callable[Concatenate[Session, _P], _T],
        *arg: _P.args,
        **kw: _P.kwargs,
    ) -> _T: ...

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context

Have a nice day!

@michaeloliverx michaeloliverx added the requires triage New issue that requires categorization label Jan 9, 2022
@zzzeek
Copy link
Member

zzzeek commented Jan 9, 2022

just a heads up that all typing will be inline for SQLAlchemy 2.0 and my current development focus is getting typing ported. the typing / structure will also be very different from sqlalchemy2-stubs as far as fundamentals, although issues like function callers like run_sync() will apply in the same way obviously.

@michaeloliverx
Copy link
Author

just a heads up that all typing will be inline for SQLAlchemy 2.0 and my current development focus is getting typing ported. the typing / structure will also be very different from sqlalchemy2-stubs as far as fundamentals, although issues like function callers like run_sync() will apply in the same way obviously.

Thanks for the info. I will leave the implementation for now. Is there a ticket to track the inline work?

@zzzeek
Copy link
Member

zzzeek commented Jan 9, 2022

yes this is currently at sqlalchemy/sqlalchemy#6810 .

There are also other related issues, I'll try to cross-link them now

@CaselIT
Copy link
Member

CaselIT commented Jan 9, 2022

I think mypy does not support pep 612 at the moment. We would need to check if id degrades gracefully (ie consider it any/any)

@CaselIT CaselIT added enhancement New feature or request and removed requires triage New issue that requires categorization labels Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants