Skip to content

TYP: remove a type: ignore #44288

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 3 commits into from
Closed

TYP: remove a type: ignore #44288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84e4db9
TYP: remove a `type: ignore`
simonjayhawkins Nov 2, 2021
6d2bcd0
fix registry.find
simonjayhawkins Nov 2, 2021
0b4d71a
Merge branch 'main' into type-ignore-1
jreback Feb 4, 2022
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
8 changes: 8 additions & 0 deletions pandas/_libs/tslibs/timestamps.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,17 @@ class Timestamp(datetime):
def utcoffset(self) -> timedelta | None: ...
def tzname(self) -> str | None: ...
def dst(self) -> timedelta | None: ...
# error: Argument 1 of "__le__" is incompatible with supertype "date";
# supertype defines the argument type as "date"
def __le__(self, other: datetime) -> bool: ... # type: ignore
# error: Argument 1 of "__lt__" is incompatible with supertype "date";
# supertype defines the argument type as "date"
def __lt__(self, other: datetime) -> bool: ... # type: ignore
# error: Argument 1 of "__ge__" is incompatible with supertype "date";
# supertype defines the argument type as "date"
def __ge__(self, other: datetime) -> bool: ... # type: ignore
# error: Argument 1 of "__gt__" is incompatible with supertype "date";
# supertype defines the argument type as "date"
def __gt__(self, other: datetime) -> bool: ... # type: ignore
# error: Signature of "__add__" incompatible with supertype "date"/"datetime"
@overload # type: ignore[override]
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/dtypes/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ def find(
dtype_type = type(dtype)
else:
dtype_type = dtype
if issubclass(dtype_type, ExtensionDtype):
if dtype_type in self.dtypes:
Copy link
Contributor

Choose a reason for hiding this comment

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

self.dtypes is actually a list. i am not 100% sure this matters but can you check (e.g. perf)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, calling find with a EA instance or EA type maybe slower if we actually check the registry which I would expect a method called find to do.

but looking some more I think we potentially have another issue.

we call Registry.find from pandas_dtype.

So the immediate thoughts are

  1. pandas.api.types.pandas_dtype is public so any changes need extra scrutiny.
  2. pandas_dtype is annotated and documented as returning a dtype instance whereas Registry.find may return an instance or a Type.
  3. pandas_dtype accepts any object and may pass that onto Registry.find so the annotations for Registry.find may need modification.

will mark as draft for now to prevent merging and try to filter type annotations for functions calling pandas_dtype to lower levels before looking at the perf implications.

>>> import pandas as pd
>>> from pandas.api.types import pandas_dtype
>>> 
>>> pandas_dtype("Int64")
Int64Dtype()
>>> 
>>> pandas_dtype(pd.Int64Dtype)
<class 'pandas.core.arrays.integer.Int64Dtype'>

according to the docs and annotations (point 2), this last statement should return an Int64Dtype instance

I think it maybe better that Registry.find only accepts strings and the other logic for EA types and instances is included in is_extension_array and pandas_dtype. will investigate further.

Copy link
Member

Choose a reason for hiding this comment

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

pandas_dtype is annotated and documented as returning a dtype instance whereas Registry.find may return an instance or a Type.

FWIW i think changing this to always return a dtype instance would fix #31356

# cast needed here as mypy doesn't know we have figured
# out it is an ExtensionDtype or type_t[ExtensionDtype]
return cast("ExtensionDtype | type_t[ExtensionDtype]", dtype)
Expand Down
6 changes: 5 additions & 1 deletion pandas/core/dtypes/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ def is_1d_only_ea_dtype(dtype: DtypeObj | None) -> bool:
)


def is_extension_array_dtype(arr_or_dtype) -> bool:
def is_extension_array_dtype(arr_or_dtype: object) -> bool:
"""
Check if an object is a pandas extension array type.

Expand Down Expand Up @@ -1476,6 +1476,10 @@ def is_extension_array_dtype(arr_or_dtype) -> bool:
return True
elif isinstance(dtype, np.dtype):
return False
elif isinstance(dtype, type) and issubclass(dtype, ExtensionDtype):
return True
elif not isinstance(dtype, str):
return False
else:
return registry.find(dtype) is not None

Expand Down
1 change: 1 addition & 0 deletions pandas/tests/arrays/test_array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ def registry_without_decimal():
def test_array_not_registered(registry_without_decimal):
# check we aren't on it
assert registry.find("decimal") is None
assert registry.find(DecimalDtype) is None
data = [decimal.Decimal("1"), decimal.Decimal("2")]

result = pd.array(data, dtype=DecimalDtype)
Expand Down