-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
TYP: handle mypy ignore in SelectionMixin #38239
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
34a6644
TYP: handle mypy ignore in SelectionMixin
ivanovmg 19f0db3
FIX: remove unnecessary import
ivanovmg d4a8a7e
TYP: use Label instead of Optional[Hashable]
ivanovmg b4dc30f
Merge branch 'master' into mypy/base
ivanovmg 48f2b08
Merge branch 'master' into mypy/base
ivanovmg bc5157d
DOC: document public attributes in SelectionMixin
ivanovmg 74f6d69
Merge branch 'master' into mypy/base
ivanovmg a68c880
Merge branch 'master' into mypy/base
ivanovmg 68a3310
Merge branch 'master' into mypy/base
ivanovmg a1df593
Merge branch 'master' into mypy/base
ivanovmg a99be97
REF: initialize exclusions in derived class
ivanovmg dcc6885
TYP: TODO on typing obj as FrameOrSeriesUnion
ivanovmg 3e025a7
Merge branch 'master' into mypy/base
ivanovmg a56f7b9
DOC: doc attributes inside class body
ivanovmg e0b1657
TYP: replace Label with Hashable
ivanovmg 4222c83
Update pandas/core/base.py
ivanovmg eb04519
Merge branch 'master' into mypy/base
ivanovmg 954a2a3
TYP: change to FrameOrSeriesUnion
ivanovmg 9f703f1
Merge branch 'master' into mypy/base
jreback e7f9028
LINT: fix trailing whitespace
ivanovmg 219eb11
Merge branch 'master' into mypy/base
ivanovmg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,9 @@ | |
Callable, | ||
Dict, | ||
FrozenSet, | ||
Hashable, | ||
Optional, | ||
Set, | ||
TypeVar, | ||
Union, | ||
cast, | ||
|
@@ -152,6 +154,9 @@ class SelectionMixin: | |
object sub-classes need to define: obj, exclusions | ||
""" | ||
|
||
obj: Any | ||
ivanovmg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
exclusions: Set[Optional[Hashable]] = set() | ||
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. Label? 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. Right! Thank you. |
||
|
||
_selection: Optional[IndexLabel] = None | ||
_internal_names = ["_cache", "__setstate__"] | ||
_internal_names_set = set(_internal_names) | ||
|
@@ -206,77 +211,41 @@ def _selection_list(self): | |
|
||
@cache_readonly | ||
def _selected_obj(self): | ||
# pandas\core\base.py:195: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
if self._selection is None or isinstance( | ||
self.obj, ABCSeries # type: ignore[attr-defined] | ||
): | ||
# pandas\core\base.py:194: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
return self.obj # type: ignore[attr-defined] | ||
else: | ||
# pandas\core\base.py:204: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
return self.obj[self._selection] # type: ignore[attr-defined] | ||
if self._selection is None or isinstance(self.obj, ABCSeries): | ||
return self.obj | ||
return self.obj[self._selection] | ||
|
||
@cache_readonly | ||
def ndim(self) -> int: | ||
return self._selected_obj.ndim | ||
|
||
@cache_readonly | ||
def _obj_with_exclusions(self): | ||
# pandas\core\base.py:209: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
if self._selection is not None and isinstance( | ||
self.obj, ABCDataFrame # type: ignore[attr-defined] | ||
): | ||
# pandas\core\base.py:217: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
return self.obj.reindex( # type: ignore[attr-defined] | ||
columns=self._selection_list | ||
) | ||
|
||
# pandas\core\base.py:207: error: "SelectionMixin" has no attribute | ||
# "exclusions" [attr-defined] | ||
if len(self.exclusions) > 0: # type: ignore[attr-defined] | ||
# pandas\core\base.py:208: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
if self._selection is not None and isinstance(self.obj, ABCDataFrame): | ||
return self.obj.reindex(columns=self._selection_list) | ||
|
||
# pandas\core\base.py:208: error: "SelectionMixin" has no attribute | ||
# "exclusions" [attr-defined] | ||
return self.obj.drop(self.exclusions, axis=1) # type: ignore[attr-defined] | ||
if len(self.exclusions) > 0: | ||
return self.obj.drop(self.exclusions, axis=1) | ||
else: | ||
# pandas\core\base.py:210: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
return self.obj # type: ignore[attr-defined] | ||
return self.obj | ||
|
||
def __getitem__(self, key): | ||
if self._selection is not None: | ||
raise IndexError(f"Column(s) {self._selection} already selected") | ||
|
||
if isinstance(key, (list, tuple, ABCSeries, ABCIndexClass, np.ndarray)): | ||
# pandas\core\base.py:217: error: "SelectionMixin" has no attribute | ||
# "obj" [attr-defined] | ||
if len( | ||
self.obj.columns.intersection(key) # type: ignore[attr-defined] | ||
) != len(key): | ||
# pandas\core\base.py:218: error: "SelectionMixin" has no | ||
# attribute "obj" [attr-defined] | ||
bad_keys = list( | ||
set(key).difference(self.obj.columns) # type: ignore[attr-defined] | ||
) | ||
if len(self.obj.columns.intersection(key)) != len(key): | ||
bad_keys = list(set(key).difference(self.obj.columns)) | ||
raise KeyError(f"Columns not found: {str(bad_keys)[1:-1]}") | ||
return self._gotitem(list(key), ndim=2) | ||
|
||
elif not getattr(self, "as_index", False): | ||
# error: "SelectionMixin" has no attribute "obj" [attr-defined] | ||
if key not in self.obj.columns: # type: ignore[attr-defined] | ||
if key not in self.obj.columns: | ||
raise KeyError(f"Column not found: {key}") | ||
return self._gotitem(key, ndim=2) | ||
|
||
else: | ||
# error: "SelectionMixin" has no attribute "obj" [attr-defined] | ||
if key not in self.obj: # type: ignore[attr-defined] | ||
if key not in self.obj: | ||
raise KeyError(f"Column not found: {key}") | ||
return self._gotitem(key, ndim=1) | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
obj
here isAny
andFrameOrSeries
in docstring.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.
Yes, this looks odd.
Initially I typed
obj
asFrameOrSeries
, but then mypy would raise errors of this kind.The reason is that
DataFrame
andSeries
interface is similar, but different.Some attributes are defined in one, but not defined in another, or the methods have different signatures.
So, I had to fallback with
Any
.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.
FrameOrSeries is a typevar, so to use it on a class/instance variable requires to make the SelectionMixin class generic. The mypy error suggests this.
if having self.obj not maintain it's type for the duration of the instance life is ok, then use FrameOrSeriesUnion instead.
mypy errors such as
pandas/core/window/rolling.py:108: error: Incompatible types in assignment (expression has type "FrameOrSeries", variable has type "Union[DataFrame, Series]") [assignment]
if doing this are where FrameOrSeries aliases are used elsewhere that also would need to change to FrameOrSeriesUnion.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.
There are 34 mypy errors of this kind (see details below) when setting to
FrameOrSeriesUnion
.Would it be a good idea for the follow up PR to fix them?
I think I can do them here, but first need to figure out how to deal with "CI Web and docs", since it complains about the missing documentation
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.
put a big TODO by the variable declaration otherwise this could get missed.
Any
should only be used where we want to allow dynamic typing ( and lose the benefits of static type checking)