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

Mypy fix/io sql #28979

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

  • 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
@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Oct 15, 2019
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.

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

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

@@ -1234,7 +1236,7 @@ class _TestSQLAlchemy(SQLAlchemyMixIn, PandasSQLTest):

"""

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

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

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
Copy link
Contributor Author

@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants