BUG/PERF: Avoid listifying in dispatch_to_extension_op
State: closed

This simplifies dispatch_to_extension_op. The remaining logic is simply
unboxing Series / Indexes in favor of their underlying arrays. This forced
two additional changes

1. Move some logic that IntegerArray relied on down to the IntegerArray ops.
   Things like handling of 0-dim ndarrays was previously broken on IntegerArray
   ops, but work with Serires[IntegerArray]
2. Fix pandas handling of 1 ** NA for object dtype (used to construct expected).

closes https://github.com/pandas-dev/pandas/issues/22922
closes https://github.com/pandas-dev/pandas/issues/22022

Labels:
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Hello @TomAugspurger! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`pandas/core/arrays/integer.py`](https://github.com/TomAugspurger/pandas/blob/044a99eceb481d620e4c81a4fc1fbb7ae0c9a461/pandas/core/arrays/integer.py) !

 - There are no PEP8 issues in the file [`pandas/core/ops.py`](https://github.com/TomAugspurger/pandas/blob/044a99eceb481d620e4c81a4fc1fbb7ae0c9a461/pandas/core/ops.py) !

 - There are no PEP8 issues in the file [`pandas/tests/arrays/test_integer.py`](https://github.com/TomAugspurger/pandas/blob/044a99eceb481d620e4c81a4fc1fbb7ae0c9a461/pandas/tests/arrays/test_integer.py) !

 - There are no PEP8 issues in the file [`pandas/tests/extension/test_integer.py`](https://github.com/TomAugspurger/pandas/blob/044a99eceb481d620e4c81a4fc1fbb7ae0c9a461/pandas/tests/extension/test_integer.py) !


Probably not, since we don’t use it. But I’ll set it to match.

________________________________
From: jbrockmendel <notifications@github.com>
Sent: Sunday, October 14, 2018 3:18:59 PM
To: pandas-dev/pandas
Cc: Tom Augspurger; Mention
Subject: Re: [pandas-dev/pandas] BUG/PERF: Avoid listifying in dispatch_to_extension_op (#23155)


@jbrockmendel commented on this pull request.

________________________________

In pandas/core/arrays/integer.py<https://github.com/pandas-dev/pandas/pull/23155#discussion_r225007235>:

> @@ -280,6 +280,8 @@ def _coerce_to_ndarray(self):
         data[self._mask] = self._na_value
         return data

+    __array_priority__ = 1  # higher than ndarray so ops dispatch to us


Index, Series, and now DataFrame all set this to 1000. Does differing from those matter?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/pandas-dev/pandas/pull/23155#pullrequestreview-164513105>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQHIg8fa88jVOc1Pkk7EGLw88WtGbmiks5uk5wzgaJpZM4XbRVy>.

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

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

```diff
@@            Coverage Diff             @@
##           master   #23155      +/-   ##
==========================================
- Coverage   92.19%   92.18%   -0.02%     
==========================================
  Files         169      169              
  Lines       50956    50967      +11     
==========================================
+ Hits        46978    46983       +5     
- Misses       3978     3984       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.6% <90.9%> (-0.01%)` | :arrow_down: |
| #single | `42.26% <9.09%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23155?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `96.68% <100%> (+0.74%)` | :arrow_up: |
| [pandas/core/arrays/integer.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVnZXIucHk=) | `95.08% <100%> (+0.17%)` | :arrow_up: |
| [pandas/core/arrays/sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3NwYXJzZS5weQ==) | `92.17% <100%> (-0.38%)` | :arrow_down: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.22% <66.66%> (-0.42%)` | :arrow_down: |
| [pandas/compat/numpy/function.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvbXBhdC9udW1weS9mdW5jdGlvbi5weQ==) | `86.66% <0%> (-0.47%)` | :arrow_down: |
| [pandas/core/dtypes/concat.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbmNhdC5weQ==) | `97.69% <0%> (-0.45%)` | :arrow_down: |
| [pandas/core/indexes/category.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9jYXRlZ29yeS5weQ==) | `97.26% <0%> (-0.28%)` | :arrow_down: |
| [pandas/core/sorting.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc29ydGluZy5weQ==) | `98.2% <0%> (-0.07%)` | :arrow_down: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `98.23% <0%> (-0.03%)` | :arrow_down: |
| ... and [8 more](https://codecov.io/gh/pandas-dev/pandas/pull/23155/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23155?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/23155?src=pr&el=footer). Last update [5e06c84...03a367e](https://codecov.io/gh/pandas-dev/pandas/pull/23155?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

> The above seems the good logic to me. But, shouldn't then the _create_comparison_method be updated to actually do this?
 

Updated SparseArray, IntegerArray, and ExtensionScalarOpsMixin to do this.

Question: should we add a test that asserts something like

```python
def test_op_with_series_is_not_implemented(self, data):
    other = pd.Series(data)
    assert data.__add__(other) is NotImplemented
```

or is that being too opinionated in our base tests?
> Question: should we add a test that asserts something like ... or is that being too opinionated in our base tests?

I think that is a good idea. People can always still override that test, if they don't want to follow it.
Deduplicated some tests and added the base test for asserting that `ExtensionArray.__add__(Series)`  returns NotImplemented.
Added docs to the MixIns and in extending.rst.

On Thu, Oct 18, 2018 at 12:34 PM Jeff Reback <notifications@github.com>
wrote:

> *@jreback* approved this pull request.
>
> just a minor documentation request. otherwise lgtm.
> ------------------------------
>
> In pandas/core/arrays/integer.py
> <https://github.com/pandas-dev/pandas/pull/23155#discussion_r226397951>:
>
> > @@ -280,7 +280,7 @@ def _coerce_to_ndarray(self):
>          data[self._mask] = self._na_value
>          return data
>
> -    __array_priority__ = 1  # higher than ndarray so ops dispatch to us
> +    __array_priority__ = 1000  # higher than ndarray so ops dispatch to us
>
> can you document this in the Mixin itself though (if you are not going to
> set it by defaulrt). It is so non-obvious that you need to do this.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/23155#pullrequestreview-166211842>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIvr4jX023yfToIfD-E47Tid1W9rEks5umLufgaJpZM4XbRVy>
> .
>

I've removed references to `__array_ufunc__`.
OK with this docs now Joris? I feel like if someone is using `__array_ufunc__` and NDArrayOperatorMixin, they probably understand it better than I do :)
Yep, looks good! (and when looking further into `__array_ufunc__` for EAs, we can always still add some notes about that)
Actually, Tom, you were right that `__array_ufunc__` is influencing the binary operations. From the docs:

> The presence of __array_ufunc__ also influences how ndarray handles binary operations like arr + obj and arr < obj when arr is an ndarray and obj is an instance of a custom class.

https://docs.scipy.org/doc/numpy-1.15.1/reference/arrays.classes.html#numpy.class.__array_ufunc__