-
-
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: boilerplate for ops internal consistency #28037
Conversation
An option for the future could make the
|
def new_method(self, other): | ||
|
||
if is_cmp and isinstance(self, ABCIndexClass) and isinstance(other, ABCSeries): | ||
# For comparison ops, Index does *not* defer to Series |
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.
For comparison ops, Index does not defer to Series
Is there a good reason for that, or just another inconsistency that ideally one time should be solved?
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 it has to do with wanting to make idx == ser
an ndarray for the sake of ser[idx == ser]
but I'm not sure
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.
For this case, ser[idx == ser]
would also work when the result is a Series
Other question for my understanding: the "unpack" in the decorator name, does that point to the unpacking of zerodim array? Or also to (future?) unpacking of Series/Index to array? |
Yes. I'm open to other names if you have a strong preference.
I hadn't thought of that, but tentatively like it. The main thing I have in mind for extending this in the future is doing a) non-array listlike coercion and b) length-match checks since we're not very consistent about either of these. |
If this is not actually unpacking our containers, I think another name would be better (I actually thought all the time those methods would be for this purpose) Just |
Sure. |
@jbrockmendel status of this |
there was a request to name something differently that i need to get around to, otherwise this should be ready (havent rebased in a while, so should do that too) |
this PR lgtm. I can't find that request, what was it? |
Addressed in a commit yesterday. It was to rename "unpack_and_defer" to "unpack_zerodim_and_defer" |
thanks @jbrockmendel |
Progress towards #23853, with more progress available pending resolution of #27911.