-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
PERF: use is_foo_dtype fastpaths #34111
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
PERF: use is_foo_dtype fastpaths #34111
Conversation
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.
Looks good to me, just one small comment
pandas/core/arrays/masked.py
Outdated
dtype = object | ||
dtype = np.dtype(object) | ||
else: | ||
dtype = pandas_dtype(dtype) |
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.
I don't think we should do this here, as this is converting to a numpy array, the dtype
can never be a pandas dtype, and is always meant to be a numpy dtype (so let numpy interpret the string / dtype-like objects)
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.
ok
pandas/core/strings.py
Outdated
@@ -74,11 +74,11 @@ def cat_core(list_of_columns: List, sep: str): | |||
""" | |||
if sep == "": | |||
# no need to interleave sep if it is empty | |||
arr_of_cols = np.asarray(list_of_columns, dtype=object) | |||
arr_of_cols = np.asarray(list_of_columns, dtype=np.dtype(object)) |
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.
You don't need to revert this to be clear, but I also don't think it's needed to change this in numpy functions
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.
fair enough; i think in this case this was a find-and-replace
updated per comments |
Thanks! |
No description provided.