Skip to content
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

Series comparison ops __finalize__ inconsistent #19271

Closed
jbrockmendel opened this issue Jan 16, 2018 · 1 comment
Closed

Series comparison ops __finalize__ inconsistent #19271

jbrockmendel opened this issue Jan 16, 2018 · 1 comment
Labels
Compat pandas objects compatability with Numpy or Python functions Duplicate Report Duplicate issue or pull request Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Comments

@jbrockmendel
Copy link
Member

jbrockmendel commented Jan 16, 2018

Looking at core.ops. _comp_method_SERIES and whether we can have the datetime and timedelta cases defer to Index subclass implementations, a couple questions come up:

https://github.com/pandas-dev/pandas/blob/master/pandas/core/ops.py#L744

    def na_op(x, y):
        # dispatch to the categorical if we have a categorical
        # in either operand
        if is_categorical_dtype(x):
            return op(x, y)
        elif is_categorical_dtype(y) and not is_scalar(y):
            return op(y, x)

This return op(y, x) looks weird. Should it be a reverse-op? e.g if op is operator.ge, should this become return operator.le(y, x)? Maybe this is irrelevant if categorical comparisons are only defined for __eq__ and __ne__? If that is the explanation, there should be a comment.

Special casing for PeriodIndex:

            if isinstance(other, ABCPeriodIndex):
                # temp workaround until fixing GH 13637
                # tested in test_nat_comparisons
                # (pandas.tests.series.test_operators.TestSeriesOperators)
                return self._constructor(na_op(self.values,
                                               other.astype(object).values),
                                         index=self.index)

I couldn't tell from #13637 what the connection was. Commenting this out did not appear to break anything in the tests. Anyone know if this is still relevant?

Update I forgot to mention the question mentioned in the Issue title. Most cases return self._constructor(res, index=self.index, name=whatever), but one case returns self._constructor(na_op(self.values, np.asarray(other)), index=self.index).__finalize__(self). Why is __finalize__ called in this case but not others?

@jreback
Copy link
Contributor

jreback commented Jan 19, 2018

duplicate of #13208

@jreback jreback closed this as completed Jan 19, 2018
@jreback jreback added Reshaping Concat, Merge/Join, Stack/Unstack, Explode Duplicate Report Duplicate issue or pull request Compat pandas objects compatability with Numpy or Python functions labels Jan 19, 2018
@jreback jreback added this to the No action milestone Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compat pandas objects compatability with Numpy or Python functions Duplicate Report Duplicate issue or pull request Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

No branches or pull requests

2 participants