-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CLN: Assorted cleanups #30260
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
CLN: Assorted cleanups #30260
Conversation
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
…n-pytables-sigs
pandas/core/dtypes/cast.py
Outdated
|
||
Parameters | ||
---------- | ||
dtype : np.dtype or ExceptionDtype |
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.
ExtensionDtype?
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.
yep, will change
@@ -74,10 +74,12 @@ def cat_core(list_of_columns: List, sep: str): | |||
""" | |||
if sep == "": | |||
# no need to interleave sep if it is empty | |||
return np.sum(list_of_columns, axis=0) | |||
arr_of_cols = np.asarray(list_of_columns, 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.
What is this cleaning up?
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.
this is for when numpy re-institutes the make-object-explicit thing from #30035.
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 think should hold off on these changes then until that comes back
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.
they reverted it specifically to give downstream time to update without breaking CIs
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.
great this is good
@@ -3314,12 +3312,12 @@ def data_orientation(self): | |||
def queryables(self) -> Dict[str, Any]: | |||
""" return a dict of the kinds allowable columns for this object """ | |||
|
|||
# mypy doesnt recognize DataFrame._AXIS_NAMES, so we re-write it here |
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.
What error was this throwing? Surprised it wouldn't see this
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 think _AXIS_NAMES is defined dynamically in a classmethod.
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.
yes, it likely needs typing, i would rather do that and remove this from here (followup ok)
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.
lgtm. small followup requests.
@@ -337,6 +337,21 @@ def changeit(): | |||
|
|||
|
|||
def maybe_promote(dtype, fill_value=np.nan): | |||
""" |
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 type at some point
@@ -686,10 +700,12 @@ def maybe_upcast(values, fill_value=np.nan, dtype=None, copy: bool = False): | |||
|
|||
Parameters | |||
---------- | |||
values : the ndarray that we want to maybe upcast | |||
values : ndarray or ExtensionArray |
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.
ideally type these at some point
@@ -74,10 +74,12 @@ def cat_core(list_of_columns: List, sep: str): | |||
""" | |||
if sep == "": | |||
# no need to interleave sep if it is empty | |||
return np.sum(list_of_columns, axis=0) | |||
arr_of_cols = np.asarray(list_of_columns, 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.
great this is good
@@ -3314,12 +3312,12 @@ def data_orientation(self): | |||
def queryables(self) -> Dict[str, Any]: | |||
""" return a dict of the kinds allowable columns for this object """ | |||
|
|||
# mypy doesnt recognize DataFrame._AXIS_NAMES, so we re-write it here |
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.
yes, it likely needs typing, i would rather do that and remove this from here (followup ok)
Many of these are broken off from #30035.