-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
TYP: Add MyPy Error Codes #35311
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
TYP: Add MyPy Error Codes #35311
Conversation
@WillAyd have added a code-check to enforce this but may not be able to enable until we bump mypy due a bug in mypy not suppressing some additional notes, so have just commented out the code check for now |
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.
Cool! Yea I think being more specific is always worthwhile
@@ -109,3 +109,4 @@ dependencies: | |||
- pip: | |||
- git+https://github.com/pandas-dev/pydata-sphinx-theme.git@master | |||
- git+https://github.com/numpy/numpydoc | |||
- pyflakes>=2.2.0 |
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 think this is available from conda?
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'll give it a go on the CI, IIRC I had issues locally with conda but later found an issue in the environment. conda list showed 2.2.0 but flake8 was still using 2.1.1
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.
conda doesn't like it.
@@ -121,7 +121,15 @@ def stringify_path( | |||
""" | |||
if hasattr(filepath_or_buffer, "__fspath__"): | |||
# https://github.com/python/mypy/issues/1424 | |||
return filepath_or_buffer.__fspath__() # type: ignore | |||
# error: Item "str" of "Union[str, Path, IO[str]]" has no attribute |
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.
What criteria are you using to include a comment or not?
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.
Hopefully, always. it makes it much easier when browsing the code for type: ignores to remove. e.g. #35344
While adding the error messages here, it was immediately apparent that those ignores removed in #35344 were masking another issue. If this was already merged to master, the error message would be in the diff (as deleted) for #35344 and make the intent of the changes more clear and hence easier to review imo.
This reverts commit 0cddba7.
@WillAyd gentle ping! |
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.
lgtm. maybe give it a day to see if @jreback has any feedback otherwise can merge
Thanks @simonjayhawkins |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff