Skip to content

TYP: Fix mypy ignores in parsers #39342

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 6 commits into from
Feb 4, 2021
Merged

Conversation

phofl
Copy link
Member

@phofl phofl commented Jan 22, 2021

  • Ensure all linting tests pass, see here for how to run them

Tried fixing the Mypy Errors in parsers

@phofl phofl added the Typing type annotations, mypy/pyright type checking label Jan 22, 2021
@jreback
Copy link
Contributor

jreback commented Jan 28, 2021

can you merge master

phofl added 2 commits January 28, 2021 22:13
…p_parsers

� Conflicts:
�	pandas/io/parsers/c_parser_wrapper.py
�	pandas/io/parsers/python_parser.py
@phofl
Copy link
Member Author

phofl commented Jan 28, 2021

Done

@simonjayhawkins Is it ok to use the | syntax?

@MarcoGorelli
Copy link
Member

quick comment on the | syntax - this is in the pyupgrade roadmap (asottile/pyupgrade#372)

@jreback jreback added this to the 1.3 milestone Jan 29, 2021
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

wow this looks good. cc @simonjayhawkins

# pandas\io\parsers.py:1559: error: Need type annotation for
# 'counts' [var-annotated]
counts = defaultdict(int) # type: ignore[var-annotated]
counts: DefaultDict[int | str, int] = defaultdict(int)
Copy link
Member

Choose a reason for hiding this comment

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

if an mi, the dict key is a tuple?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, thx. I looked them through again and fixed 2 more I think

# pandas\io\parsers.py:1559: error: Need type annotation for
# 'counts' [var-annotated]
counts = defaultdict(int) # type: ignore[var-annotated]
counts: DefaultDict[int | str, int] = defaultdict(int)
Copy link
Member

Choose a reason for hiding this comment

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

if an mi, the dict key is a tuple?

@pep8speaks
Copy link

pep8speaks commented Feb 3, 2021

Hello @phofl! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-02-03 18:52:52 UTC

@phofl
Copy link
Member Author

phofl commented Feb 3, 2021

ping @simonjayhawkins

Comment on lines 552 to 561
self, col_indices: List[int], names: List[Union[int, str]]
self, col_indices: List[int], names: List[int | str | tuple]
Copy link
Member

Choose a reason for hiding this comment

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

see #39538 for some discussion on this syntax change - perhaps it's best to defer it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will roll this back then

Copy link
Member Author

Choose a reason for hiding this comment

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

Rolled back

@jreback jreback merged commit ca4f204 into pandas-dev:master Feb 4, 2021
@jreback
Copy link
Contributor

jreback commented Feb 4, 2021

thanks @phofl

@phofl phofl deleted the typ_parsers branch February 4, 2021 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants