Skip to content

TYP: remove mypy ignore from array_manager.py #52249

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

Merged
merged 6 commits into from
Mar 29, 2023
Merged

TYP: remove mypy ignore from array_manager.py #52249

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4a9bcb6
TYP: remove ignore from array_manager.py
natmokval Mar 27, 2023
90a5972
Merge branch 'main' into 37715-remove-mypy-ignore
natmokval Mar 27, 2023
ca58222
Merge branch 'main' into 37715-remove-mypy-ignore
natmokval Mar 28, 2023
31287f0
TYP: remove ignore from array_manager.py II
natmokval Mar 28, 2023
c861a74
TYP: remove ignore from array_manager.py III
natmokval Mar 29, 2023
73c4ad7
Merge branch 'main' into 37715-remove-mypy-ignore
natmokval Mar 29, 2023
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
4 changes: 1 addition & 3 deletions pandas/core/internals/array_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,9 +842,7 @@ def iset(
else:
assert isinstance(loc, np.ndarray)
assert loc.dtype == "bool"
# error: Incompatible types in assignment (expression has type "ndarray",
# variable has type "range")
indices = np.nonzero(loc)[0] # type: ignore[assignment]
indices = np.array([i for i in range(len(loc)) if loc[i] is True])
Copy link
Member

Choose a reason for hiding this comment

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

I think you would need to change line 837 to indices : range | np.ndarray = range(

Mypy uses early binding, the if block declares the variable for the first time with type range. When mypy parses the else block, it thinks the type should be range and not np.ndarray. I believe pyright doesn't do that (for each block it uses its own type and then uses the union after an if/else)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you would need to change line 837 to indices : range | np.ndarray = range(

Thank you for the comment. I did as you suggested.

Mypy uses early binding, the if block declares the variable for the first time with type range. When mypy parses the else block, it thinks the type should be range and not np.ndarray. I believe pyright doesn't do that (for each block it uses its own type and then uses the union after an if/else)

Could it be the reason why locally I didn't have a failure in test_dup_across_dtypes(self), but in ci there was an error in the same function in line pandas/tests/frame/test_nonunique_indexes.py:147: ?

Copy link
Member

Choose a reason for hiding this comment

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

Could it be the reason why locally I didn't have a failure in test_dup_across_dtypes(self), but in ci there was an error in the same function in line pandas/tests/frame/test_nonunique_indexes.py:147: ?

The type annotations shouldn't cause a test to fail at runtime. I'm not familiar with the test but I assume that the code changes from your previous commits might have changed the runtime behavior which triggered some tests to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I get it. np.nonzero works much more effectively regarding time than my code changes.


assert value.ndim == 2
assert value.shape[0] == len(self._axes[0])
Expand Down