BUG: fix Series(extension array) + extension array values addition
State: closed

- [x] closes #22478
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Bug
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
It is a simple fix, but not sure where to put additional tests.
> It is a simple fix, but not sure where to put additional tests.

I would look in `pandas/tests/extension/base/ops.py`, there is a `test_arith_series_with_array` that tests Series + Series, you could add in that test to do a `Series + array` (with a similar `self.check_opname` call). But make sure you also test that it actually fails without your fix.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=h1) Report
> Merging [#22479](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/2be2ba570453ab09f0c8da46dfd8e1a179771fee?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `0%`.

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

```diff
@@           Coverage Diff           @@
##           master   #22479   +/-   ##
=======================================
  Coverage   92.04%   92.04%           
=======================================
  Files         169      169           
  Lines       50740    50740           
=======================================
  Hits        46705    46705           
  Misses       4035     4035
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.45% <0%> (ø)` | :arrow_up: |
| #single | `42.24% <0%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `96.71% <0%> (ø)` | :arrow_up: |

------

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

pandas/tests/extension/base/ops.py Can not be discovered by pytest. How to test it ? @jorisvandenbossche 
@peterpanmj the actual tests are in one directory up (they inherit from those base classes), so if you do `pytest pandas/tests/extension`, they should all be run.
can you update
Hello @peterpanmj! Thanks for updating the PR.

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

 - There are no PEP8 issues in the file [`pandas/tests/extension/integer/test_integer.py`](https://github.com/peterpanmj/pandas/blob/0108553e04126a8e1268c921efcd631e35314ea7/pandas/tests/extension/integer/test_integer.py) !


I will compose a test shortly @jreback 
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=h1) Report
> Merging [#22479](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/b0f9a104f323d687a56ea878ff78ff005f37b42d?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #22479      +/-   ##
==========================================
+ Coverage   92.18%   92.19%   +<.01%     
==========================================
  Files         169      169              
  Lines       50833    50830       -3     
==========================================
- Hits        46862    46861       -1     
+ Misses       3971     3969       -2
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.6% <100%> (ø)` | :arrow_up: |
| #single | `42.37% <0%> (+0.01%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `97.51% <100%> (+0.09%)` | :arrow_up: |
| [pandas/core/indexes/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9wZXJpb2QucHk=) | `93.48% <0%> (-0.18%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.66% <0%> (-0.12%)` | :arrow_down: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.39% <0%> (-0.11%)` | :arrow_down: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `98.1% <0%> (-0.02%)` | :arrow_down: |
| [pandas/core/panel.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcGFuZWwucHk=) | `97.89% <0%> (-0.02%)` | :arrow_down: |
| [pandas/core/sparse/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3Nlcmllcy5weQ==) | `95.22% <0%> (-0.02%)` | :arrow_down: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.74% <0%> (-0.01%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.2% <0%> (-0.01%)` | :arrow_down: |
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `95.45% <0%> (-0.01%)` | :arrow_down: |
| ... and [8 more](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22479?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/22479?src=pr&el=footer). Last update [b0f9a10...7efeed8](https://codecov.io/gh/pandas-dev/pandas/pull/22479?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/22479?src=pr&el=h1) Report
> Merging [#22479](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/249f0ee72a7c6bc6260423c8293f62e44dd4ff1a?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #22479      +/-   ##
==========================================
+ Coverage   92.17%   92.17%   +<.01%     
==========================================
  Files         169      169              
  Lines       50721    50721              
==========================================
+ Hits        46750    46751       +1     
+ Misses       3971     3970       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.58% <100%> (ø)` | :arrow_up: |
| #single | `42.34% <0%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22479?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/22479/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `97.07% <100%> (+0.13%)` | :arrow_up: |

------

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

@jreback  Any thing I need to change ?
u can add this issue number to the integer_array whatsnew 
Shouldn’t be necessary since there isn’t a released version with this bug.

________________________________
From: Jeff Reback <notifications@github.com>
Sent: Sunday, September 30, 2018 11:55:26 AM
To: pandas-dev/pandas
Cc: Tom Augspurger; Comment
Subject: Re: [pandas-dev/pandas] BUG: fix Series(extension array) + extension array values addition (#22479)


u can add this issue number to the integer_array whatsnew

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<https://github.com/pandas-dev/pandas/pull/22479#issuecomment-425734775>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQHIgc6-J2xZWQTPk0lszWPfNU2s8KSks5ugPd9gaJpZM4WI2Md>.

that’s not the point
it provides a reference to all issues with this feature

just add the number (on the existing note)
Sorry I don't see the need. If this bug hadn't been there when you wrote IntegerArray, would you have made a note saying that ops between Series[IntegerArray] and IntegeryArray works? I don't see why we should distract readers going through the release notes with the fact that this was implemented across two PRs.
i think u have missed my point

we list all issues that are relevent if possible . all u do is add it to the list of issues. no extra note is necessary. 

the problem with ignoring it is if you only look at the listed issues you will miss things. it’s irrelevant whether it’s a bug or additional features. it’s all a part of the single what’s new note. 

this has been practice for many years

this way you have a complete record of what has changed.
How about we leave the bug there in 0.24 . Fix it later in 0.24.1 and update the whatsnew then.
> How about we leave the bug there in 0.24 . Fix it later in 0.24.1 and update the whatsnew then.

No, we certainly want to get this fixed in master / in the first coming release.

---

I agree with Tom, this is something that only failed in master, for a new feature that doesn't exist in 0.23. So no whatsnew note is needed, and adding the issue number there only adds noise IMO. If it would have been another issue that already existed before that was also resolved by the same fix, yes, then it would be good to list the issue number in the whatsnew.

(but let's further discuss this on gitter / mail to not bother this PR?)

---

> @jreback is there an issue for not listifying right? That seems unnecessary when new_left and right are both EAs of the same type.

I didn't find one. Opened https://github.com/pandas-dev/pandas/issues/22922


Note that this is still tested via decimal.
Thanks @peterpanmj and @TomAugspurger !
this was not fully reviewed

need tests for the reverse op as well, eg

data + series

ideally we would test using the fixture for all ops as well
@jreback can you open a new issue with specifics?

> need tests for the reverse op as well, eg

That hits a different code path, so would be a different bug if it doesn't work (what's the expected result? Right now IntegerArray + Series[IntegerArray] returns an IntegerArray)

> ideally we would test using the fixture for all ops as well

I meant to include a note with my push, but was distracted by the float array issue. In this case, I opted for a simpler test that didn't include all the indirection of `_check_op_name`.  This test was about covering our dispatching, so just the add is sufficient.
> what's the expected result? Right now IntegerArray + Series[IntegerArray] returns an IntegerArray

That should be a Series instead IMO (that's what we do with Series/array as well, and Series is "higher in the hierarchy")
https://github.com/pandas-dev/pandas/issues/22974

On Wed, Oct 3, 2018 at 6:45 AM Joris Van den Bossche <
notifications@github.com> wrote:

> what's the expected result? Right now IntegerArray + Series[IntegerArray]
> returns an IntegerArray
>
> That should be a Series instead IMO (that's what we do with Series/array
> as well, and Series is "higher in the hierarchy")
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22479#issuecomment-426606636>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIooH7Vl03undKUDVvG_cdan8cO_dks5uhKNGgaJpZM4WI2Md>
> .
>