Skip to content

Accept sqlalchemy.engine.Engine for SQL IO API (read_sql, to_sql) #281

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

Merged
merged 3 commits into from
Sep 8, 2022

Conversation

wakabame
Copy link
Contributor

@wakabame wakabame commented Sep 8, 2022

Outline

The typecheck for sql io api developed in 4c97ed3 does not accept sqlalchemy.engine.Engine for the argument con.

However, the original pandas implementation accepts them

In fact, pandasSQL_builder, which is used in to_sql and read_sql, requires sqlalchemy.engine.Connectable not only sqlalchemy.engine.Connection:
https://github.com/pandas-dev/pandas/blob/50c119dce9005cb3e49c0cfb89f396aeecab94f1/pandas/io/sql.py#L757

So, in this PR, I improve the typecheck wider, and introduce testcodes for the case of sqlalchemy connection.

Check List

  • Closes #xxxx (Replace xxxx with the Github issue number)
    • No issues appeared
  • Tests added: Please use assert_type() to assert the type of any return value

This is my first pull request for this repository, so I apologize if there are any problems. I would appreciate it if you could check it.

@bashtage
Copy link
Contributor

bashtage commented Sep 8, 2022

LGTM. Let's make sure it passes.

@wakabame wakabame force-pushed the feature/accept_engine branch from 09cad1e to 4ff5da1 Compare September 8, 2022 12:52
@wakabame wakabame force-pushed the feature/accept_engine branch from 4ff5da1 to 3f8d44d Compare September 8, 2022 12:56
@wakabame wakabame requested a review from Dr-Irv September 8, 2022 12:56
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @wakabame

@Dr-Irv Dr-Irv merged commit 28fd624 into pandas-dev:main Sep 8, 2022
@wakabame wakabame deleted the feature/accept_engine branch September 8, 2022 22:54
@wakabame
Copy link
Contributor Author

wakabame commented Sep 9, 2022

@Dr-Irv
Thank you for your quick and kind comment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants