BUG: Series.combine() fails with ExtensionArray inside of Series
State: closed

- [x] closes #20825
closes #21248
- [x] tests added / passed
   - extension/category/test_categorical.py:test_combine
   - extension/decimal/test_decimal.py:test_combine
   - series/test_combine_concat.py:test_combine_scalar
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry
   - modified `whatsnew.v0.24.0.txt`

This is a split of #20889 to just fix the `combine` issue.




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

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21183?src=pr&el=h1) Report
> Merging [#21183](https://codecov.io/gh/pandas-dev/pandas/pull/21183?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/0c65c57a279e755ab7093db925d1e580f9878dae?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/21183/graphs/tree.svg?width=650&height=150&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/21183?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #21183      +/-   ##
==========================================
+ Coverage   91.85%   91.85%   +<.01%     
==========================================
  Files         153      153              
  Lines       49549    49573      +24     
==========================================
+ Hits        45512    45537      +25     
+ Misses       4037     4036       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.25% <100%> (ø)` | :arrow_up: |
| #single | `41.85% <7.14%> (-0.03%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21183?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/21183/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.17% <100%> (+0.05%)` | :arrow_up: |
| [pandas/core/reshape/melt.py](https://codecov.io/gh/pandas-dev/pandas/pull/21183/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZWx0LnB5) | `97.34% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/21183/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.8% <0%> (+0.02%)` | :arrow_up: |
| [pandas/io/formats/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/21183/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvc3R5bGUucHk=) | `96.12% <0%> (+0.08%)` | :arrow_up: |
| [pandas/core/dtypes/missing.py](https://codecov.io/gh/pandas-dev/pandas/pull/21183/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL21pc3NpbmcucHk=) | `92.52% <0%> (+0.57%)` | :arrow_up: |

------

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

Hello @Dr-Irv! Thanks for updating the PR.

















Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on June 07, 2018 at 15:45 Hours UTC
@jreback You write:

> I would rather not do this here at all, prefering instead to dispatch to the EA itself for a `.combine()` method, this will be much simpler.

Are you suggesting that there is a default implementation of `.combine()` in `ExtensionArray` that can be overridden by implementers of the EA subclass, and that `Series.combine()` dispatch to that?  If so, that sounds reasonable to me, and I can make that change.
> Are you suggesting that there is a default implementation of .combine() in ExtensionArray that can be overridden by implementers of the EA subclass, and that Series.combine() dispatch to that? If so, that sounds reasonable to me, and I can make that change.

no I would suggest the default impl is NotImplemented, this is getting too messy here with all of these if/thens
Lgtm otherwise. 
@Dr-Irv also can you make sure that the cases in your OP are covered here (I think so, but pls verify)
@jreback wrote:

> @Dr-Irv also can you make sure that the cases in your OP are covered here (I think so, but pls verify)

Here you go:

```python
In [1]: import pandas as pd

In [2]: from pandas.tests.extension.decimal.array import DecimalArray, make_dat
   ...: a

In [3]: da1= make_data()

In [4]: da2 = make_data()

In [5]: s1 = pd.Series(DecimalArray(da1))

In [6]: s2 = pd.Series(DecimalArray(da2))

In [7]: s1.combine(s2, lambda x1, x2: x1 if x1 < x2 else x2)
Out[7]:
0     0.08098492909619514623642544393078424036502838...
1     0.38231358539972493115755014514434151351451873...
2     0.28683621751796128940270591556327417492866516...
3     0.68315691040978210324396968644578009843826293...
4     0.37188279135446988821200875463546253740787506...
5     0.29391070580371847498213355720508843660354614...
6     0.00281662212378319676275850724778138101100921...
7     0.68424426414243522120983698187046684324741363...
8     0.82298917655718306640721948497230187058448791...
9       0.104520052984668154749670065939426422119140625
10    0.36541026835682677287309161329176276922225952...
11    0.15450295421108961591016850434243679046630859375
12    0.28227045216414337058807859648368321359157562...
13    0.07428041682799846334717130957869812846183776...
14    0.76112360264534761888910452398704364895820617...
15    0.31991420667129122357152937183855101466178894...
16    0.43808559346706621440148410329129546880722045...
17    0.81185756018222499097447553140227682888507843...
18    0.18755848298291555309447176114190369844436645...
19    0.45064358633799639353156862853211350739002227...
20    0.01908941180337764276231382609694264829158782...
21    0.05994588216042351369594598509138450026512145...
22    0.45670931996552022180679841767414472997188568...
23    0.21696297680411980035586338999564759433269500...
24    0.77792921984939356061516946283518336713314056...
25    0.10732191741882657343154505724669434130191802...
26    0.26112839619732719498301776184234768152236938...
27    0.78917786400767853116633432364324107766151428...
28    0.11041422426646729793020540455472655594348907...
29    0.48501200782261966182318246865179389715194702...
                            ...
70    0.01573854854098000188855621672701090574264526...
71    0.44412972763803593156950455522746779024600982...
72    0.25386583135624463114510263039846904575824737...
73    0.18052436704533048050791421701433137059211730...
74    0.01768961191491325024571779067628085613250732...
75    0.19783353177328266703227654943475499749183654...
76    0.64275305832443418996291484290850348770618438...
77    0.56556945132078839666434078026213683187961578...
78    0.45692610540309075428666574225644581019878387...
79    0.54208092770195082099604633185663260519504547...
80    0.04475845682326173857745743589475750923156738...
81    0.02586933176340200368770183558808639645576477...
82    0.38495809393040258949980625402531586587429046...
83    0.09473826636927540345567422264139167964458465...
84    0.09800918702868577359055279885069467127323150...
85    0.06037114142095312274705065647140145301818847...
86    0.54181647584734216049895394462510012090206146...
87    0.39820409063224215806542360951425507664680480...
88    0.08573080156333134915769278450170531868934631...
89    0.31841090690861806322686788917053490877151489...
90    0.05991365875060672419039065061951987445354461...
91    0.20527392489879703330046822884469293057918548...
92    0.68151285114935533648861110123107209801673889...
93    0.33294634130492617440921776506002061069011688...
94    0.50598008087709389624109235228388570249080657...
95    0.31112635294260559959411693853326141834259033...
96    0.15173983856659867264227159466827288269996643...
97    0.12529127428765418628131556033622473478317260...
98    0.15472130908820258543556747099501080811023712...
99    0.36042221705147114985123835140257142484188079...
Length: 100, dtype: decimal

In [8]: cat1 = pd.Categorical(values=["one","two","three","three","two","one"],
   ...:
   ...:    ...:  categories=["one","two","three"], ordered=True)
   ...:    ...: cat2 = pd.Categorical(values=["three","two","one","one","two","
   ...: three"],
   ...:    ...:  categories=["one","two","three"], ordered=True)
   ...:    ...: s1 = pd.Series(cat1)
   ...:    ...: s2 = pd.Series(cat2)
   ...:    ...: s1, s2
   ...:
Out[8]:
(0      one
 1      two
 2    three
 3    three
 4      two
 5      one
 dtype: category
 Categories (3, object): [one < two < three], 0    three
 1      two
 2      one
 3      one
 4      two
 5    three
 dtype: category
 Categories (3, object): [one < two < three])

In [9]:  s1.combine(s2, lambda x1, x2: x1 <= x2)
Out[9]:
0     True
1     True
2    False
3    False
4     True
5     True
dtype: bool

In [10]: s = pd.Series([i*10 for i in range(5)])

In [11]: s.combine(22, lambda x,y: min(x,y))
Out[11]:
0     0
1    10
2    20
3    22
4    22
dtype: int64

In [12]: pd.__version__
Out[12]: '0.24.0.dev0+76.g2a21117c7.dirty'
```

i mean can u verify that u have test coverage for those cases
@jreback wrote:

> i mean can u verify that u have test coverage for those cases

The new tests subsume those cases. Should I explicitly include those cases?

> The new tests subsume those cases. Should I explicitly include those cases?

As long the case is part of the tests you added, that is fine.
Regarding `combine` now working for categorical data while in 0.23.0 it failed (https://github.com/pandas-dev/pandas/pull/21183#discussion_r193441379), I think that is fine. 
At least that is the consequence of catching the TypeError possibly raised by `ExtensionArray._from_sequence`; which allows the output has another dtype as in the input from combine.
thanks @Dr-Irv nice patch!

thanks for the patience. non-trivial things take more time :-D