Split fastpath IntegerArray constructor and general purpose constructor
State: closed

Related to the discussion we were having on the PR: https://github.com/pandas-dev/pandas/pull/21160/#discussion_r201209246 (@jreback @shoyer).

Opening this PR for discussion, to *show* what I meant there, to hopefully make it more concrete discussion.

Briefly the changes / rationale:

- Separate the "fastpath" and general purpose constructor (in master everything is passed through `coerce_to_array`:
  - `IntegerArray` is the fastpath if you know you have a proper `values` and `mask`
  - `integer_array` as general purpose constructor function, that eg parses lists, detects NaNs etc (exact API to discuss of course)
- IMO this clear separation makes the code cleaner (in all internal usage we know what we want). In the past we often added a `fastpath` keyword to the constructor, but IMO we should not repeat that pattern in new code (it is overloading the main constructor by combining things).
- This also has the benefit of improved performance. Eg on an array of 1e5 values, the current constructor has an overhead of ca 80 µs, not much of course but that means that a simple slice is 82 instead of 2 µs (of course, this improvement does not need the exact proposal in this PR, you could certainly do a PR adding a fastpath keyword that achieves the same boost).



Note that this is not yet a fully implemented PR:

- Docstrings still need to be updated
- Tests for new behaviour needs to be added
- `coerce_to_array` can be simplified a little bit, as currently the  `mask` argument is nowhere used (unless we foresee use cases for that?) 

Labels:
name: Needs Discussion
description: Requires discussion from core team before further action
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
looks interesting - will look tomorrow
+1 on having a fastpath-like constructor, +1 on it not being accessed via a `fastpath` kwarg.  -0.5 on introducing a new naming convention for constructors.

We don't need to require it of downstream EA authors, but for pandas-internal EAs we should have some kind of consistent patterns for EA constructors.  Since `_simple_new`+`_shallow_copy` are already well-established (and used all over the Datetime/Timedelta/Period EA mixins), why not stick with that?
@jbrockmendel good questions. 

`_shallow_copy` is in this case not fully applicable I think, as `IntegerArray` currently has no metadata (which I think is the purpose of `_shallow_copy` to make a copy but pass through metadata). At least for now, I don't think there is a use case for it in the IntegerArray implementation. 
But of course, if we start sharing code or for other reasons, I might make sense to add such a method at some point (which for IntegerArray would be identical to the fastpath constructor at the moment). But if we want to generalize that, we might need to consider expanding the EA interface for this (if the need comes up).

For `_simple_new`: historically, we have overloaded the class constructors to do many things (and exposed as the main user API entry-point), but personally I think it is a better design to have designated method/functions for such constructors (which is of course debatable). In the EA interface we already have `_from_sequence` for some part of this general constructor from values, because we decided that we don't want to pin the class constructor on doing this. 
So that leaves open the possibility to use the class constructor for the fastpath / `_simple_new`, so why not use it? I think in general, such a fastpath constructor / `_simple_new` would only be used within the class itself (eg the IntervalIndex or other interval-related functionality outside the array never calls the fastpath of IntervalArray), so we are kind of free what to do here. 

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

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

```diff
@@            Coverage Diff             @@
##           master   #22070      +/-   ##
==========================================
- Coverage   92.05%   92.05%   -0.01%     
==========================================
  Files         169      169              
  Lines       50709    50719      +10     
==========================================
+ Hits        46679    46688       +9     
- Misses       4030     4031       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.46% <100%> (-0.01%)` | :arrow_down: |
| #single | `42.24% <7.4%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22070?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/22070/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL19faW5pdF9fLnB5) | `100% <ø> (ø)` | :arrow_up: |
| [pandas/core/arrays/integer.py](https://codecov.io/gh/pandas-dev/pandas/pull/22070/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVnZXIucHk=) | `94.48% <100%> (-0.19%)` | :arrow_down: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/22070/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.72% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/22070/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.43% <100%> (ø)` | :arrow_up: |

------

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

So the failures is because I now changed the code so that a list of numpy scalars is always coerced to int64 (before it took the type of the scalars). 
Such a change is consistent with eg Series, but then we need to more thoroughly pass through the `dtype` to `_from_sequence` in the tests to get this passing.
@jreback can you have a look at this again?
yes
ping
thanks @jorisvandenbossche nice improvement
Thanks!