-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Adjust tests in array folder for new string option #56188
Conversation
cc @mroeschke gentle ping |
pandas/core/algorithms.py
Outdated
@@ -929,10 +931,13 @@ def value_counts_internal( | |||
|
|||
# For backwards compatibility, we let Index do its normal type | |||
# inference, _except_ for if if infers from object to bool. | |||
idx = Index(keys) | |||
idx = Index(keys, dtype=input_dtype if input_dtype != "float16" else None) |
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.
Should these changes not be here after your comments in #56187?
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.
oh very good point, forgot the revert. thx. does
This reverts commit f570a4f
@@ -932,7 +932,10 @@ def value_counts_internal( | |||
idx = Index(keys) | |||
if idx.dtype == bool and keys.dtype == object: | |||
idx = idx.astype(object) | |||
elif idx.dtype != keys.dtype: | |||
elif ( | |||
idx.dtype != keys.dtype # noqa: PLR1714 # # pylint: disable=R1714 |
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.
Just confirming this change isn't related to your referenced, closed PR right?
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.
No this is fine, this is the infer_string inference, which will still happen
Thanks @phofl |
sits on top of #56187