Skip to content
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

remove HashableT in frame.pyi where possible #1104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dr-Irv
Copy link
Collaborator

@Dr-Irv Dr-Irv commented Jan 31, 2025

mypy has finally put in the feature allowing asymmetric getters/setters. During the testing, they had an issue with our stubs regarding where we use HashableT. See python/mypy#18510 (comment)

This fixes that issue. Then once mypy 1.16 i released, we can fix the tests for setting columns that have # type: ignore in it.

@Dr-Irv Dr-Irv requested a review from twoertwein January 31, 2025 15:38
pandas-stubs/core/frame.pyi Show resolved Hide resolved
pandas-stubs/core/frame.pyi Show resolved Hide resolved
@@ -432,10 +431,10 @@ class DataFrame(NDFrame, OpsMixin, _GetItemHack):
self,
index: _bool = ...,
column_dtypes: (
_str | npt.DTypeLike | Mapping[HashableT1, npt.DTypeLike] | None
_str | npt.DTypeLike | Mapping[Hashable, npt.DTypeLike] | None
Copy link
Member

Choose a reason for hiding this comment

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

will this accept dict[str, npt.DTypeLike]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, tested in line 3090 in test_frame.py

Copy link
Member

Choose a reason for hiding this comment

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

it only works because of bidirectional type inference, if you change the test slightly:

+    dtypes = {"col1": np.int8, "col2": np.int16}
     check(
-        assert_type(
-            DF.to_records(False, {"col1": np.int8, "col2": np.int16}), np.recarray
-        ),
+        assert_type(DF.to_records(False, dtypes), np.recarray),
         np.recarray,
     )

it fails

tests/test_frame.py:3090: error: Argument 2 to "to_records" of "DataFrame" has incompatible type "dict[str, type[object]]"; expected "str | dtype[Any] | type[Any] | _SupportsDType[dtype[Any]] | str | tuple[Any, int] | tuple[Any, SupportsIndex | Sequence[SupportsIndex]] | list[Any] | _DTypeDict | tuple[Any, Any] | None | Mapping[Hashable, dtype[Any] | type[Any] | _SupportsDType[dtype[Any]] | str | tuple[Any, int] | tuple[Any, SupportsIndex | Sequence[SupportsIndex]] | list[Any] | _DTypeDict | tuple[Any, Any] | None] | None" [arg-type]

/Users/twoertwein/pandas-stubs/tests/test_frame.py:3090:42 - error: Argument of type "dict[str, int8 | int16]" cannot be assigned to parameter "column_dtypes" of type "DTypeLike | Mapping[Hashable, DTypeLike]" in function "to_records"

For some reason the other list[str] case was fine, maybe setter are handed differently?

pandas-stubs/core/frame.pyi Show resolved Hide resolved
@Dr-Irv
Copy link
Collaborator Author

Dr-Irv commented Feb 1, 2025

@twoertwein I've addressed all your comments. I don't think any other changes are necessary. Let me know if you disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants