Skip to content

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

Merged
merged 16 commits into from
Mar 17, 2021
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
TYP: dtypes.cast
  • Loading branch information
jbrockmendel committed Mar 15, 2021
commit 1ee41ae3db822afd8278de53373341add183f988
21 changes: 8 additions & 13 deletions pandas/core/dtypes/cast.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,21 +407,16 @@ def maybe_cast_result(

assert not is_scalar(result)

if (
isinstance(dtype, ExtensionDtype)
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
if isinstance(dtype, ExtensionDtype):
if not is_categorical_dtype(dtype) and dtype.kind != "M":
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The 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?

it does look sketchy, yah

Copy link
Member

Choose a reason for hiding this comment

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

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.

# We have to special case categorical so as not to upcast
# things like counts back to categorical

cls = dtype.construct_array_type()
result = maybe_cast_to_extension_array(cls, result, dtype=dtype)
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:
# 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]
elif (numeric_only and is_numeric_dtype(dtype)) or not numeric_only:
result = maybe_downcast_to_dtype(result, dtype)

return result

Expand Down