-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
WIP for MyPy CI Integration #25622
WIP for MyPy CI Integration #25622
Changes from all commits
f46e165
d358ab5
db12d9f
b77347a
4c68a7b
caec323
fb987f4
9b5daaa
d169cb6
2abc6f5
2807e00
fb41b5f
ac7e768
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
[mypy] | ||
ignore_missing_imports=True | ||
follow_imports=silent | ||
|
||
[mypy-pandas.conftest,pandas.tests.*] | ||
ignore_errors=True |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
pandas/core/dtypes/base.py | ||
pandas/core/groupby/groupby.py | ||
pandas/core/internals/managers.py | ||
pandas/core/common.py | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this not possible in the setup file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea I was hoping that 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/timedeltas.py | ||
pandas/core/arrays/datetimes.py | ||
pandas/core/arrays/base.py | ||
pandas/core/frame.py | ||
pandas/core/indexes/base.py |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,11 @@ class providing the base-class of operations. | |
from functools import partial, wraps | ||
import types | ||
import warnings | ||
from typing import FrozenSet, Optional, Type | ||
|
||
import numpy as np | ||
|
||
from pandas._libs import Timestamp, groupby as libgroupby | ||
from pandas._libs import Timestamp, groupby as libgroupby # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See note on Cython imports |
||
import pandas.compat as compat | ||
from pandas.compat import range, set_function_name, zip | ||
from pandas.compat.numpy import function as nv | ||
|
@@ -325,7 +326,7 @@ def _group_selection_context(groupby): | |
|
||
class _GroupBy(PandasObject, SelectionMixin): | ||
_group_selection = None | ||
_apply_whitelist = frozenset() | ||
_apply_whitelist = frozenset() # type: FrozenSet[str] | ||
|
||
def __init__(self, obj, keys=None, axis=0, level=None, | ||
grouper=None, exclusions=None, selection=None, as_index=True, | ||
|
@@ -1041,7 +1042,7 @@ def _bool_agg(self, val_test, skipna): | |
""" | ||
|
||
def objs_to_bool(vals): | ||
# type: (np.ndarray) -> (np.ndarray, typing.Type) | ||
# type: (np.ndarray) -> (np.ndarray, Type) | ||
if is_object_dtype(vals): | ||
vals = np.array([bool(x) for x in vals]) | ||
else: | ||
|
@@ -1050,7 +1051,7 @@ def objs_to_bool(vals): | |
return vals.view(np.uint8), np.bool | ||
|
||
def result_to_bool(result, inference): | ||
# type: (np.ndarray, typing.Type) -> np.ndarray | ||
# type: (np.ndarray, Type) -> np.ndarray | ||
return result.astype(inference, copy=False) | ||
|
||
return self._get_cythonized_result('group_any_all', self.grouper, | ||
|
@@ -1743,7 +1744,7 @@ def quantile(self, q=0.5, interpolation='linear'): | |
""" | ||
|
||
def pre_processor(vals): | ||
# type: (np.ndarray) -> (np.ndarray, Optional[typing.Type]) | ||
# type: (np.ndarray) -> (np.ndarray, Optional[Type]) | ||
if is_object_dtype(vals): | ||
raise TypeError("'quantile' cannot be performed against " | ||
"'object' dtypes!") | ||
|
@@ -1758,7 +1759,7 @@ def pre_processor(vals): | |
return vals, inference | ||
|
||
def post_processor(vals, inference): | ||
# type: (np.ndarray, Optional[typing.Type]) -> np.ndarray | ||
# type: (np.ndarray, Optional[Type]) -> np.ndarray | ||
if inference: | ||
# Check for edge case | ||
if not (is_integer_dtype(inference) and | ||
|
@@ -2021,7 +2022,7 @@ def _get_cythonized_result(self, how, grouper, aggregate=False, | |
Function to be applied to result of Cython function. Should accept | ||
an array of values as the first argument and type inferences as its | ||
second argument, i.e. the signature should be | ||
(ndarray, typing.Type). | ||
(ndarray, Type). | ||
**kwargs : dict | ||
Extra arguments to be passed back to Cython funcs | ||
|
||
|
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.
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
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.
To be clear when running this from the project root you would do
mypy @mypy_whitelist.txt
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.
@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 checkedThere 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.
@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.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.
Thanks for the feedback. Was that done per package / module in the config file?
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.
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:
However, I don't remember if it worked or not.