Skip to content

WIP for MyPy CI Integration #25622

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

Closed
wants to merge 13 commits into from
Closed

WIP for MyPy CI Integration #25622

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f46e165
Updated gitignore to exclude mypy cache
WillAyd Mar 10, 2019
d358ab5
Added initial mypy configuration files
WillAyd Mar 10, 2019
db12d9f
dtypes/base.py cleanup
WillAyd Mar 10, 2019
b77347a
Cleaned up groupby.groupby
WillAyd Mar 10, 2019
4c68a7b
Fixed most of internals.blocks
WillAyd Mar 10, 2019
caec323
internals.manager fixup
WillAyd Mar 10, 2019
fb987f4
core.common fixup
WillAyd Mar 10, 2019
9b5daaa
First cleanup pass for pandas.core.arrays
WillAyd Mar 10, 2019
d169cb6
pandas.core cleanups
WillAyd Mar 10, 2019
2abc6f5
pandas.core.indexes cleanup
WillAyd Mar 10, 2019
2807e00
Alternate configuration
WillAyd Mar 11, 2019
fb41b5f
Updated whitelist
WillAyd Mar 11, 2019
ac7e768
Reverted newline deletion
WillAyd Mar 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Added initial mypy configuration files
  • Loading branch information
@WillAyd
WillAyd committed Mar 10, 2019
commit d358ab55e6f83335f28bc072458e9390cc42a97e
8 changes: 8 additions & 0 deletions mypy.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[mypy]
follow_imports=silent
Copy link
Member Author

Choose a reason for hiding this comment

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

This is less than ideal but without this the errors are out of control. My motivation for setting this comes from the Mypy documentation:

https://mypy.readthedocs.io/en/latest/running_mypy.html#following-imports

To wit:

If you are planning on adding type hints to a large, existing code base, we recommend you start by trying to make your entire codebase (including files that do not use type hints) pass under --follow-imports=normal. This is usually not too difficult to do: mypy is designed to report as few error messages as possible when it is looking at unannotated code.

If doing this is intractable, we recommend passing mypy just the files you want to type check and use --follow-imports=silent. Even if mypy is unable to perfectly type check a file, it can still glean some useful information by parsing it (for example, understanding what methods a given object has). See Using mypy with an existing codebase for more recommendations.


[mypy-numpy.*]
ignore_missing_imports=True

[mypy-pandas._libs.*]
Copy link
Member Author

Choose a reason for hiding this comment

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

Generally doesn't look like Mypy likes Cython imports. Have this in the config but also noted specifically on imports within the code.

There's probably a better approach to this just haven't found yet

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the PEP, you should be able to use .pyi/stub files to annotate compiled extensions. Not ideal though since they'd have to stay in-sync with the extension

ignore_missing_imports=True
17 changes: 17 additions & 0 deletions mypy_whitelist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
pandas/core/dtypes/base.py
Copy link
Member Author

Choose a reason for hiding this comment

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

These are all of the files that currently have some sort of hints in them. The motivation for this comes form this part in the documentation:

https://mypy.readthedocs.io/en/latest/running_mypy.html#reading-a-list-of-files-from-a-file

So thinking for initial CI runs we can whitelist the modules we want to run against, though this is ideally just a temporary file

Copy link
Member Author

Choose a reason for hiding this comment

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

To be clear when running this from the project root you would do mypy @mypy_whitelist.txt

Copy link
Member Author

Choose a reason for hiding this comment

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

@Naddiseo any chance you have experience with this? Reading from docs I think suggested approach will be to whitelist particular modules at the outset and slowly open up as more are added.

I'd like to avoid having two files to control configuration here but I don't see an easy way in the .ini file to control which modules actually get checked

Copy link
Contributor

Choose a reason for hiding this comment

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

@WillAyd, not with the whitelist approach. I went with the blacklist approach instead, and had a bunch of modules and packages with ignore_errors set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback. Was that done per package / module in the config file?

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently I'm just ignoring packages. But, I think at one point I was doing both, and I seem to remember doing a mixture where I'd ignore everything in a package except a specific file.
I think it looked like:

[mypy-package.subpackage]
ignore_errors=True
[mypy-package.subpackage.module]
ignore_errors=False

However, I don't remember if it worked or not.

pandas/core/groupby/groupby.py
pandas/core/internals/blocks.py
pandas/core/internals/managers.py
pandas/core/common.py
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this not possible in the setup file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea I was hoping that Mypy_path in the ini file would work but didn't have any luck. I didn't also see a very good way to make this readable therein since the ini would expect this to be a comma separated list.

With that said this may just be temporary anyway - would ideally like to run on the entire code. Modules without existing types would be ignored by default anyway

pandas/core/arrays/datetimes.py
pandas/core/arrays/integer.py
pandas/core/arrays/period.py
pandas/core/arrays/sparse.py
pandas/core/arrays/datetimelike.py
pandas/core/arrays/array_.py
pandas/core/arrays/base.py
pandas/core/frame.py
pandas/core/base.py
pandas/core/indexes/period.py
pandas/core/indexes/datetimelike.py
pandas/core/indexes/base.py