Skip to content

Mypy fix/io sql #28979

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

Mypy fix/io sql #28979

wants to merge 0 commits into from

Conversation

@aaditya-panik
Copy link
Contributor

@aaditya-panik aaditya-panik commented Oct 14, 2019

  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

As part of this PR, did the following:

  • Variable typing.

  • Code cleanup and reformatting the imports.

  • Internal class _TestSQLAlchemyConn behaves like an abstract class.

  • To solve variable name clashing between parent classes in some of the errors, I restructured the inheritance from a structure like

    (_TestSQLAlchemy, _TestDBSQLAlchemy) --> TestDBSQLAlchemy

    to

    _TestSQLAlchemy --> _TestDBSQLAlchemy --> TestDBSQLAlchemy

    Not sure why it was like that unless I'm missing something?
@gfyoung gfyoung added Compat pandas objects compatability with Numpy or Python functions IO SQL to_sql, read_sql, read_sql_query Testing pandas testing functions or related to the test suite labels Oct 15, 2019
@aaditya-panik aaditya-panik mentioned this pull request Oct 15, 2019
Closed
4 tasks
@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Oct 15, 2019
@simonjayhawkins simonjayhawkins mentioned this pull request Oct 15, 2019
Closed
38 tasks
simonjayhawkins
simonjayhawkins requested changes Oct 15, 2019
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

@aaditya-panik Thanks for the PR.

I think is you make the changes suggested, all the other changes can be reverted.
pandas/tests/io/test_sql.py Outdated
flavor = "sqlite"
mode = None
flavor = "sqlite" # type: Optional[str]
mode = None # type: Optional[str]
Copy link
Member

@simonjayhawkins simonjayhawkins Oct 15, 2019

Choose a reason for hiding this comment

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

Suggested change
mode = None # type: Optional[str]
mode = None # type: str
pandas/tests/io/test_sql.py Outdated
"""

flavor = None
flavor = None # type: Optional[str]
Copy link
Member

@simonjayhawkins simonjayhawkins Oct 15, 2019

Choose a reason for hiding this comment

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

Suggested change
flavor = None # type: Optional[str]
flavor = None # type: str
@aaditya-panik
Copy link
Contributor Author

aaditya-panik commented Oct 15, 2019

I will be creating and linking a new PR because I think I botched my forked repo branch. Will link the two PRs once I create the new one with the changes @simonjayhawkins suggested.
@aaditya-panik aaditya-panik mentioned this pull request Oct 15, 2019
Merged
3 tasks
@aaditya-panik
Copy link
Contributor Author

aaditya-panik commented Oct 15, 2019

@simonjayhawkins made the changes you suggested. Let me know if that works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compat pandas objects compatability with Numpy or Python functions IO SQL to_sql, read_sql, read_sql_query Testing pandas testing functions or related to the test suite Typing type annotations, mypy/pyright type checking

3 participants

@aaditya-panik @simonjayhawkins @gfyoung