-
-
Notifications
You must be signed in to change notification settings - Fork 141
upgrade SQLAlchemy and pyright #687
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
Conversation
@@ -1095,6 +1095,7 @@ def test_read_sql_via_sqlalchemy_connection(): | |||
assert_type(read_sql("select * from test", con=conn), DataFrame), | |||
DataFrame, | |||
) | |||
engine.dispose() |
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.
Can engine
be used as a context manager? Might be slightly nicer if that's possible
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.
No, it is not possible.
| sqlalchemy.sql.expression.Selectable | ||
| sqlalchemy.sql.expression.TextClause | ||
| sqlalchemy.sql.Select | ||
| FromStatement |
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.
Do we need to add those libraries (or their stubs) as a dependency?
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 don't think so. I just added the two new types that seem to be valid for the test we had. We have sqlalchemy
in our dev environment, but we don't want to make it something you have to install to use the stubs.
Thanks @Dr-Irv ! |
Based on pandas-dev/pandas#50558 (comment), we could upgrade SqlAlchemy to 2.0. Doing that revealed an issue in those tests and some of the typing.
Also bumped pyright to 1.1.306, but the default value for
useLibraryCodeForTypes
changed in that version, so added that inpyproject.toml
. Without that, some of the plotting tests fail becausematplotlib
doesn't have typing yet.