-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Further annotate sqlalchemy.engine and collections #6680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read about half of this in detail.
url: str | ||
dialect: Dialect | ||
logging_name: str # only exists if not None during initialization | ||
echo: echo_property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be ClassVar
, because the class has an instance of echo_property
. Or just echo: _EchoFlag
, since the underlying echo_property
instance is an implementation detail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how well class vars play together with the descriptor protocol. @property
is usually not wrapped in ClassVar
either.
I just tried to use just echo: _EchoFlag
, and while it should work, stubtest complains, so I opted for the version that requires less stubtest overrides.
It would be nice to see |
That's my plan after migrating the stubs from https://github.com/srittau/python-stubs/tree/master/stubs/sqlalchemy. I know that the latter work for my use case. :) |
(I will look at the other comments later/in the next few days.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking so long to review the rest of this.
A couple thoughts:
- There now seems to be 4 different sqlalchemy stubs out there:
sqlalchemy-stubs
, whatever zzzeek works on (my official "why can't there be nested generics" question typing#999), https://github.com/srittau/python-stubs, and the stubs in typeshed. Is this really a good thing? - This PR took a lot of time to review. I would have appreciated splitting it up into smaller pull requests.
No description provided.