-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: fix ignores #40452
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
TYP: fix ignores #40452
Changes from all commits
6759676
2f1ae91
1ee41ae
e9b59d1
6277b44
453d289
fa6bcfd
2e5ea07
28c4733
26a4fb0
b17b8c4
411e3e3
5a29047
801ea35
4e8cb5f
8adca0c
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 |
---|---|---|
|
@@ -44,7 +44,6 @@ | |
) | ||
from pandas._libs.tslibs.timedeltas import array_to_timedelta64 | ||
from pandas._typing import ( | ||
AnyArrayLike, | ||
ArrayLike, | ||
Dtype, | ||
DtypeObj, | ||
|
@@ -407,27 +406,16 @@ def maybe_cast_result( | |
|
||
assert not is_scalar(result) | ||
|
||
if ( | ||
is_extension_array_dtype(dtype) | ||
and not is_categorical_dtype(dtype) | ||
and dtype.kind != "M" | ||
): | ||
# We have to special case categorical so as not to upcast | ||
# things like counts back to categorical | ||
|
||
# error: Item "dtype[Any]" of "Union[dtype[Any], ExtensionDtype]" has no | ||
# attribute "construct_array_type" | ||
cls = dtype.construct_array_type() # type: ignore[union-attr] | ||
# error: Argument "dtype" to "maybe_cast_to_extension_array" has incompatible | ||
# type "Union[dtype[Any], ExtensionDtype]"; expected "Optional[ExtensionDtype]" | ||
result = maybe_cast_to_extension_array( | ||
cls, result, dtype=dtype # type: ignore[arg-type] | ||
) | ||
if isinstance(dtype, ExtensionDtype): | ||
if not is_categorical_dtype(dtype) and dtype.kind != "M": | ||
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. The logic has changed here slightly. is there a latent bug here that needs a test. 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. looking some more, maybe the type annotations for maybe_downcast_to_dtype are incorrect? 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. since this is only called from one place and that is with numeric_only=True (ought to remove that kwarg) and the two cases excluded here dont have is_numeric_dtype(dtype), the behavior will end up being the same before and after 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.
it does look sketchy, yah 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.
so it does. |
||
# We have to special case categorical so as not to upcast | ||
# things like counts back to categorical | ||
|
||
elif numeric_only and is_numeric_dtype(dtype) or not numeric_only: | ||
# error: Argument 2 to "maybe_downcast_to_dtype" has incompatible type | ||
# "Union[dtype[Any], ExtensionDtype]"; expected "Union[str, dtype[Any]]" | ||
result = maybe_downcast_to_dtype(result, dtype) # type: ignore[arg-type] | ||
cls = dtype.construct_array_type() | ||
result = maybe_cast_to_extension_array(cls, result, dtype=dtype) | ||
|
||
elif (numeric_only and is_numeric_dtype(dtype)) or not numeric_only: | ||
result = maybe_downcast_to_dtype(result, dtype) | ||
|
||
return result | ||
|
||
|
@@ -549,17 +537,23 @@ def maybe_upcast_putmask(result: np.ndarray, mask: np.ndarray) -> np.ndarray: | |
new_dtype = ensure_dtype_can_hold_na(result.dtype) | ||
|
||
if new_dtype != result.dtype: | ||
# error: Argument 1 to "astype" of "_ArrayOrScalarCommon" has incompatible | ||
# type "Union[dtype[Any], ExtensionDtype]"; expected "Union[dtype[Any], | ||
# None, type, _SupportsDType, str, Union[Tuple[Any, int], Tuple[Any, | ||
# Union[int, Sequence[int]]], List[Any], _DTypeDict, Tuple[Any, Any]]]" | ||
result = result.astype(new_dtype, copy=True) # type: ignore[arg-type] | ||
result = result.astype(new_dtype, copy=True) | ||
|
||
np.place(result, mask, np.nan) | ||
|
||
return result | ||
|
||
|
||
@overload | ||
def ensure_dtype_can_hold_na(dtype: np.dtype) -> np.dtype: | ||
... | ||
|
||
|
||
@overload | ||
def ensure_dtype_can_hold_na(dtype: ExtensionDtype) -> ExtensionDtype: | ||
... | ||
|
||
|
||
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. could perhaps create a _DtypeObjT typvar (local to the module if we don't need elsewhere) instead of an overload here since ensure_dtype_can_hold_na is type-preserving 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. i considered that and ended up not pulling the trigger, would be OK with it if you feel strongly about it 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.
I wouldn't block on this, so i'll leave it to you. I won't be able to fully review this PR till tomorrow, so the comments so far are just some bits that stood out. |
||
def ensure_dtype_can_hold_na(dtype: DtypeObj) -> DtypeObj: | ||
""" | ||
If we have a dtype that cannot hold NA values, find the best match that can. | ||
|
@@ -636,9 +630,7 @@ def _maybe_promote(dtype: np.dtype, fill_value=np.nan): | |
|
||
kinds = ["i", "u", "f", "c", "m", "M"] | ||
if is_valid_na_for_dtype(fill_value, dtype) and dtype.kind in kinds: | ||
# error: Incompatible types in assignment (expression has type | ||
# "Union[dtype[Any], ExtensionDtype]", variable has type "dtype[Any]") | ||
dtype = ensure_dtype_can_hold_na(dtype) # type: ignore[assignment] | ||
dtype = ensure_dtype_can_hold_na(dtype) | ||
fv = na_value_for_dtype(dtype) | ||
return dtype, fv | ||
|
||
|
@@ -1471,7 +1463,7 @@ def soft_convert_objects( | |
|
||
|
||
def convert_dtypes( | ||
input_array: AnyArrayLike, | ||
input_array: ArrayLike, | ||
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. This is now inconsistent with the docstring 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. updated |
||
convert_string: bool = True, | ||
convert_integer: bool = True, | ||
convert_boolean: bool = True, | ||
|
@@ -1483,7 +1475,7 @@ def convert_dtypes( | |
|
||
Parameters | ||
---------- | ||
input_array : ExtensionArray, Index, Series or np.ndarray | ||
input_array : ExtensionArray or np.ndarray | ||
convert_string : bool, default True | ||
Whether object dtypes should be converted to ``StringDtype()``. | ||
convert_integer : bool, default True | ||
|
@@ -1707,15 +1699,10 @@ def maybe_cast_to_datetime( | |
# GH 25843: Remove tz information since the dtype | ||
# didn't specify one | ||
|
||
# error: Item "ndarray" of "Union[ndarray, DatetimeArray]" | ||
# has no attribute "tz" | ||
if dta.tz is not None: # type: ignore[union-attr] | ||
if dta.tz is not None: | ||
# equiv: dta.view(dtype) | ||
# Note: NOT equivalent to dta.astype(dtype) | ||
|
||
# error: Item "ndarray" of "Union[ndarray, | ||
# DatetimeArray]" has no attribute "tz_localize" | ||
dta = dta.tz_localize(None) # type: ignore[union-attr] | ||
dta = dta.tz_localize(None) | ||
value = dta | ||
elif is_datetime64tz: | ||
dtype = cast(DatetimeTZDtype, dtype) | ||
|
@@ -1725,38 +1712,19 @@ def maybe_cast_to_datetime( | |
# be localized to the timezone. | ||
is_dt_string = is_string_dtype(value.dtype) | ||
dta = sequence_to_datetimes(value, allow_object=False) | ||
# error: Item "ndarray" of "Union[ndarray, DatetimeArray]" | ||
# has no attribute "tz" | ||
if dta.tz is not None: # type: ignore[union-attr] | ||
# error: Argument 1 to "astype" of | ||
# "_ArrayOrScalarCommon" has incompatible type | ||
# "Union[dtype[Any], ExtensionDtype, None]"; expected | ||
# "Union[dtype[Any], None, type, _SupportsDType, str, | ||
# Union[Tuple[Any, int], Tuple[Any, Union[int, | ||
# Sequence[int]]], List[Any], _DTypeDict, Tuple[Any, | ||
# Any]]]" | ||
value = dta.astype( | ||
dtype, copy=False # type: ignore[arg-type] | ||
) | ||
if dta.tz is not None: | ||
value = dta.astype(dtype, copy=False) | ||
elif is_dt_string: | ||
# Strings here are naive, so directly localize | ||
# equiv: dta.astype(dtype) # though deprecated | ||
|
||
# error: Item "ndarray" of "Union[ndarray, | ||
# DatetimeArray]" has no attribute "tz_localize" | ||
value = dta.tz_localize( # type: ignore[union-attr] | ||
dtype.tz | ||
) | ||
value = dta.tz_localize(dtype.tz) | ||
else: | ||
# Numeric values are UTC at this point, | ||
# so localize and convert | ||
# equiv: Series(dta).astype(dtype) # though deprecated | ||
|
||
# error: Item "ndarray" of "Union[ndarray, | ||
# DatetimeArray]" has no attribute "tz_localize" | ||
value = dta.tz_localize( # type: ignore[union-attr] | ||
"UTC" | ||
).tz_convert(dtype.tz) | ||
value = dta.tz_localize("UTC").tz_convert(dtype.tz) | ||
elif is_timedelta64: | ||
# if successful, we get a ndarray[td64ns] | ||
value, _ = sequence_to_td64ns(value) | ||
|
@@ -1789,14 +1757,12 @@ def maybe_cast_to_datetime( | |
elif value.dtype == object: | ||
value = maybe_infer_to_datetimelike(value) | ||
|
||
elif not isinstance(value, ABCExtensionArray): | ||
elif isinstance(value, list): | ||
# only do this if we have an array and the dtype of the array is not | ||
# setup already we are not an integer/object, so don't bother with this | ||
# conversion | ||
|
||
# error: Argument 1 to "maybe_infer_to_datetimelike" has incompatible type | ||
# "Union[ExtensionArray, List[Any]]"; expected "Union[ndarray, List[Any]]" | ||
value = maybe_infer_to_datetimelike(value) # type: ignore[arg-type] | ||
value = maybe_infer_to_datetimelike(value) | ||
|
||
return value | ||
|
||
|
@@ -1974,10 +1940,8 @@ def construct_1d_arraylike_from_scalar( | |
except OutOfBoundsDatetime: | ||
dtype = np.dtype(object) | ||
|
||
if is_extension_array_dtype(dtype): | ||
# error: Item "dtype" of "Union[dtype, ExtensionDtype]" has no | ||
# attribute "construct_array_type" | ||
cls = dtype.construct_array_type() # type: ignore[union-attr] | ||
if isinstance(dtype, ExtensionDtype): | ||
cls = dtype.construct_array_type() | ||
subarr = cls._from_sequence([value] * length, dtype=dtype) | ||
|
||
else: | ||
|
@@ -1994,11 +1958,7 @@ def construct_1d_arraylike_from_scalar( | |
elif dtype.kind in ["M", "m"]: | ||
value = maybe_unbox_datetimelike(value, dtype) | ||
|
||
# error: Argument "dtype" to "empty" has incompatible type | ||
# "Union[dtype, ExtensionDtype]"; expected "Union[dtype, None, type, | ||
# _SupportsDtype, str, Tuple[Any, int], Tuple[Any, Union[int, | ||
# Sequence[int]]], List[Any], _DtypeDict, Tuple[Any, Any]]" | ||
subarr = np.empty(length, dtype=dtype) # type: ignore[arg-type] | ||
subarr = np.empty(length, dtype=dtype) | ||
subarr.fill(value) | ||
|
||
return subarr | ||
|
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.
with these Literal[True]/Literal[False] may also need to add an extra bool case. ie. duplicate the function signature as an additional overload (follow-on ok as may not be needed if this is internal) see #40200 and #40197
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.
yah this shouldnt be user-facing