-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
REF: use sanitize_array in Index.__new__ #49718
Conversation
@MarcoGorelli are you the right person to complain to about this? i hereby complain. |
yup, removing this one now |
# cast to list here | ||
data = list(data) | ||
if len(data) == 0: | ||
# unlike Series, we default to object 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.
Would we want to align this in the future?
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 have no strong opinion on this
@@ -208,6 +206,22 @@ | |||
_dtype_obj = np.dtype("object") | |||
|
|||
|
|||
def _wrapped_sanitize(cls, data, dtype: DtypeObj | None, copy: bool): |
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.
So ideally we wouldn't need this in the future once there's more alignment between Series and Index?
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.
the strict_ints keyword we could get rid of after #49609. the rest of this wrapping is mostly about giving Index-specific exception messages, so is pretty benign
(actually, at the time i refactored this function out i was calling it 3ish times within Index.__new__
. now that its only called once, we could inline it)
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.
Inlining can happen in a follow up
Thanks @jbrockmendel |
* REF: Index.__new__ use sanitize_array * REF: _wrapped_sanitize * re-use wrapped_sanitize * cln * REF: share * avoid extra copy * troubleshoot CI * pylint fixup
* REF: Index.__new__ use sanitize_array * REF: _wrapped_sanitize * re-use wrapped_sanitize * cln * REF: share * avoid extra copy * troubleshoot CI * pylint fixup
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.i.e. share code paths with Series constructor.
This isn't Technically Correct until #49348