Catch Exception in combine
State: closed

Closes https://github.com/pandas-dev/pandas/issues/22850

Labels:
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/series.py`](https://github.com/TomAugspurger/pandas/blob/fdd43c4de98992a6f97a835fdbb525f829ef1d69/pandas/core/series.py) !

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


Ah, import issue on py27.

We probably shouldn't be shadowing names of builtin packages in our extension tests. I'm going to rename decimal and json to decimal_array and json_array.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22936?src=pr&el=h1) Report
> Merging [#22936](https://codecov.io/gh/pandas-dev/pandas/pull/22936?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/fea27f0736a4b8f6626da60a6abc2f6e26b8a365?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/22936/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/22936?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #22936      +/-   ##
==========================================
+ Coverage   92.19%   92.19%   +<.01%     
==========================================
  Files         169      169              
  Lines       50821    50822       +1     
==========================================
+ Hits        46852    46855       +3     
+ Misses       3969     3967       -2
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.61% <100%> (ø)` | :arrow_up: |
| #single | `42.36% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22936?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/22936/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `95.71% <100%> (+1.46%)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/22936/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.75% <100%> (ø)` | :arrow_up: |

------

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

The release note I just added is deliberately vague. I don't want non-EA authors poking around in here :)
+1 on the renaming; need caffeine for the rest
Updates:

1. Moved the test with the rest extension ops tests in `test_decimal.py` (though I still think we need a home for non-EA specific, non-interface tests). Maybe that's just `pandas/tests/test_ops.py` though. 
2. Undid the module renames (not a bad idea to do on its own though)
3. Changed ExtensionScalarOpsMixin to catch exceptions in `_from_sequence`, and return an ndarray in that case. Added a test for that (previously untested AFAICT). Docs coming in #22932 

This and https://github.com/pandas-dev/pandas/pull/22932 (divmod) are both touching the same lines now, sorry. I'd recommend merging this one first.
Updates:

1. Return an ndarray instead of a list, even when `coerce_to_dtype=False`
2. Documented the expected return type. Are people familiar with the mypy syntax? `-> Callable[[Any, Any], Union[ndarray, ExtenionsionArray]` means the return value is a callable taking two arguments. Right now that's just Any and Any, since there could be things like `EA + 1`. All we know is that one of them is an EA. The last item in `Callable` is the return value of the callable, which is either an ndarray or EA.
3. Added links back here around the broad `except Exception`s
thanks @TomAugspurger 

not sure I completely understand the use of ``UserDict`` in the json/array, but you did leave a TODO
The UserDict goes back to the original implementation. We have a few `if isinstance(thing, dict)` internally that break things for `JSONArray`. We'll need to sort that if / when we properly support nested data, where the "scalar" type for an array is actually a sequence like a dict or array.