Allow operator overloading for custom classes to return custom objects
State: closed

Uses a similar convention as numpy's ufunc overriding mechanism (https://docs.scipy.org/doc/numpy-1.13.0/neps/ufunc-overrides.html) to fix #22255

- [x] closes #22255
- [ ] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Enhancement

Comments:
Sorry, I missed the issue when it was originally opened. My initial reaction is that a `__pandas_ufunc__` isn't appropriate; I don't think it's defined well enough.

@jbrockmendel do you have any thoughts?
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=h1) Report
> Merging [#26652](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e25fd0d8ab10d6cc4dfe0f5808976f7921512c9f?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `36.11%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26652/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26652      +/-   ##
==========================================
- Coverage   91.88%   91.83%   -0.05%     
==========================================
  Files         174      174              
  Lines       50683    50719      +36     
==========================================
+ Hits        46568    46577       +9     
- Misses       4115     4142      +27
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.37% <36.11%> (-0.04%)` | :arrow_down: |
| #single | `41.76% <8.33%> (-0.14%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `91.8% <36.11%> (-2.89%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97% <0%> (-0.12%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=footer). Last update [e25fd0d...53404f2](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=h1) Report
> Merging [#26652](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e25fd0d8ab10d6cc4dfe0f5808976f7921512c9f?src=pr&el=desc) will **decrease** coverage by `50.09%`.
> The diff coverage is `8.33%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26652/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26652      +/-   ##
==========================================
- Coverage   91.88%   41.78%   -50.1%     
==========================================
  Files         174      174              
  Lines       50683    50719      +36     
==========================================
- Hits        46568    21194   -25374     
- Misses       4115    29525   +25410
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `?` | |
| #single | `41.78% <8.33%> (-0.11%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `50.27% <8.33%> (-44.42%)` | :arrow_down: |
| [pandas/io/formats/latex.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvbGF0ZXgucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/sas/sas\_constants.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfY29uc3RhbnRzLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/core/groupby/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9jYXRlZ29yaWNhbC5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvcGxvdHRpbmcucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/formats/html.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvaHRtbC5weQ==) | `0% <0%> (-99.37%)` | :arrow_down: |
| [pandas/io/sas/sas7bdat.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXM3YmRhdC5weQ==) | `0% <0%> (-91.16%)` | :arrow_down: |
| [pandas/io/sas/sas\_xport.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfeHBvcnQucHk=) | `0% <0%> (-90.1%)` | :arrow_down: |
| [pandas/core/sparse/scipy\_sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3NjaXB5X3NwYXJzZS5weQ==) | `10.14% <0%> (-89.86%)` | :arrow_down: |
| ... and [129 more](https://codecov.io/gh/pandas-dev/pandas/pull/26652/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=footer). Last update [e25fd0d...65f82fc](https://codecov.io/gh/pandas-dev/pandas/pull/26652?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

I would want to see some compelling use cases for this before going forward.

Something close that I _could_ get on board with is something like `__array_priority__` to define how `op(EA_foo, EA_bar)` behaves.
Thank you for your feedbacks @jreback and @TomAugspurger. I think your concerns are valid. 

In order to resolve your concern, I think a better alternative is to return `NotImplemented` in those situations that pandas does not handle `other` object. For example, pandas is raising `TypeError` in some situations like line 2306 of:

https://github.com/pandas-dev/pandas/blob/e0c41f79104c5bc61952c9a14f1883cd5bda53f7/pandas/core/ops.py#L2292-L2307

If pandas does not raise and return NotImplemented instead, python itself with raise `TypeError` when the other object can't handle it. Take the following example:

```python
>>> class A:
...     def __add__(self, other):
...         if isinstance(other, A):
...             return A()
...         return NotImplemented
... 
>>> A() + A()
<__main__.A object at 0x7f803dfe5c18>
>>> A() + object()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'A' and 'object'
```

In this proposal, there is no longer need to define inappropriate hooks like `__pandas_ufunc__`, and at the same time, python defers implementation to third-party libraries if pandas returns `NotImplemented` in the "else" part of its type checks like line 2306.

Finally, I think there are some use cases like "lazy/distributed/GPU" evaluation that can take benefit from custom overloading of operators in interaction with pandas objects.
@jbrockmendel The only concrete use case I know is my own library, https://github.com/sspipe/sspipe. However, not blocking third-party interaction with pandas objects makes ideas like lazy/distributed/remote computation libraries to interact with pandas objects too. If numpy had blocked its operators to its own objects, pandas could not become so convenient. 
On the other hand, in my last proposal at the comment above, the code change will be limited to returning `NotImplemented` instead of raising `TypeError`. This doesn't add any explicit/incoherent logic like `__pandas_ufunc__` hook to the existing code.
> if pandas returns NotImplemented in the "else" part of its type checks like line 2306.

I think that's a starting point. Would you be able to summarize the conditions under which you think we should return NotImplemented, but currently don't?
@TomAugspurger In most of the cases, we can replace the `else: <ORIGINAL_ELSE>` part of type checkings in `ops.py` with something like this:

```python
builtin_types = (int, float, complex, list, tuple, range, str, datetime, timedelta)
pandas_operand_types = (np.generic, np.array, ABCGeneric) + builtin_types

...
elif isinstance(other, pandas_operand_types):
    <ORIGINAL_ELSE>
else:
    return NotImplemented
```

One case that I think this will break current behavior is for Series whose dtype is `object`. In this case, the current behavior is to iterate over its items and apply operation on them item-wise, independent of the operand's datatype. Although I believe this behavior is too magical, I think in order to maintain current behavior, we can use the following snippet for this case:

```python
reverse_operations = {
    f'__{op}__': f'__r{op}__' for op in ('add', 'sub', 'mul', 'mod', 'pow', 'truediv', 'floordiv', 'divmod'
}

...
else:
    try:
        other_handler = getattr(other, reverse_operations[op.__name__])
        return other_handler(self)
    except Exception:
         <ORIGINAL_ELSE>
```

I'm not sure what is best here. The definition of the "right" thing to do
here is unclear to me.

On Thu, Jun 6, 2019 at 3:54 PM Mohammad Hossein Sekhavat <
notifications@github.com> wrote:

> @TomAugspurger <https://github.com/TomAugspurger> In most of the cases,
> we can replace the else: <ORIGINAL_ELSE> part of type checkings in ops.py
> to something like this:
>
> builtin_types = (int, float, complex, list, tuple, range, str, datetime, timedelta)
> pandas_operand_types = (np.generic, np.array, ABCGeneric) + builtin_types
> ...elif isinstance(other, pandas_operand_types):
>     <ORIGINAL_ELSE>else:
>     return NotImplemented
>
> One case that I think this will break current behavior is for Series whose
> dtype is object. In this case, the current behavior is to iterate over
> its items and apply operation on them item-wise, independent of the
> operand's datatype. Although I believe this behavior is too magical, I
> think in order to maintain current behavior, we can use the following
> snippet for this case:
>
> reverse_operations = {
>     f'__{op}__': f'__r{op}__' for op in ('add', 'sub', 'mul', 'mod', 'pow', 'truediv', 'floordiv', 'divmod'
> }
> ...else:
>     try:
>         other_handler = getattr(other, reverse_operations[op.__name__])
>         return other_handler(self)
>     except Exception:
>          <ORIGINAL_ELSE>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/26652?email_source=notifications&email_token=AAKAOIQYRHHVTU5ZETIR3S3PZF2ORA5CNFSM4HTMZ23KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXEEGKI#issuecomment-499663657>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIWOM7PGS2ULGMIFUPLPZF2ORANCNFSM4HTMZ23A>
> .
>

closing this for now in favor of potentially more discussion on issue #22255 not sure we want to go down this route at this point (adding api for __pandas_ufunc__, though its interesting.)