TYP: fix ignores #40452
TYP: fix ignores #40452
Conversation
| def ensure_dtype_can_hold_na(dtype: ExtensionDtype) -> ExtensionDtype: | ||
| ... | ||
|
|
||
|
|
There was a problem hiding this comment.
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.
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.
if you feel strongly about it
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.
| ) -> Union[np.ndarray, DatetimeArray]: | ||
| ... | ||
|
|
||
|
|
There was a problem hiding this comment.
may not be needed if this is internal
yah this shouldnt be user-facing
| 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.
The logic has changed here slightly. is there a latent bug here that needs a test.
There was a problem hiding this comment.
looking some more, maybe the type annotations for maybe_downcast_to_dtype are incorrect?
There was a problem hiding this comment.
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.
looking some more, maybe the type annotations for maybe_downcast_to_dtype are incorrect?
it does look sketchy, yah
There was a problem hiding this comment.
and the two cases excluded here dont have is_numeric_dtype(dtype), the behavior will end up being the same before and after
so it does.
There was a problem hiding this comment.
lgtm. making good progress clearing down the numpy related ignores. Thanks @jbrockmendel .
| @@ -1471,7 +1463,7 @@ def soft_convert_objects( | |||
|
|
|||
|
|
|||
| def convert_dtypes( | |||
| input_array: AnyArrayLike, | |||
| input_array: ArrayLike, | |||
There was a problem hiding this comment.
This is now inconsistent with the docstring
|
Thanks @jbrockmendel |
No description provided.