CLN: unify __finalize__ treatment for Series ops #28590
Conversation
| result = finalizer(result) | ||
|
|
||
| # Set the result's name after finalizer is called because finalizer | ||
| # would set it back to self.name |
There was a problem hiding this comment.
Can you move this comment inside the construct_result function?
| lambda x: x.__finalize__(self) | ||
| if isinstance(other, (np.ndarray, ABCIndexClass)) | ||
| else x | ||
| ) |
There was a problem hiding this comment.
Now it will always be calling finalize. So was the comment there wrong? What if other is a Series? Does this have an impact on the result?
There was a problem hiding this comment.
Does this have an impact on the result?
In the overwhemingly-most-common case where there is no user-defined _metadata, no, this doesn't affect the result. Otherwise, it affects the result when other is a Series.
|
any additional tests needed? |
I'm inclined towards "no" because the behavior that is affected is stuff we don't particularly care about. |
|
Just to verify, we don't have any cases where we pass a DataFrame to On my disallow-duplicates I noticed some strange behavior with DataFrame's having a |
Not in this file, no. |
|
LGTM then.
…On Tue, Sep 24, 2019 at 11:10 AM jbrockmendel ***@***.***> wrote:
Just to verify, we don't have any cases where we pass a DataFrame to
Series.*finalize* right?
Not in this file, no.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#28590?email_source=notifications&email_token=AAKAOIUHL3G7RCOKTJFBBX3QLI3YNA5CNFSM4IZXYXJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7O5DPQ#issuecomment-534630846>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKAOIXHGA2V44FSS3LV66DQLI3YNANCNFSM4IZXYXJA>
.
|
|
thanks! |
ATM we have three slightly different usages for the arithmetic, comparison and logical ops, with no clear reason for the differences. This changes that to make the arithmetic the one true version, which has the added benefit of letting us share _construct_result code across these three functions.