ENH: nullable Float32/64 ExtensionArray
State: closed

As proposed at https://github.com/pandas-dev/pandas/issues/32265#issuecomment-627004139, this is adding a float extension dtype and array (for now avoiding/ignoring the NaN-discussion), using the masked array approach following our other nullable dtypes.

Brief overview:

- This PR adds a `FloatingArray` and `Float32Dtype` / `Float64Dtype` (similar as for the nullable integer dtypes)
- I am using the "Float64" pattern with capital "F" as name for the dtype to distinguish the string name of the "float64" dtype from numpy (consistent with Int64 etc)
- For now I only added Float32 and Float64 (we can see later if we want to add others like float16)
- Upon conversion of input (both in construction as in operations such as setitem), any `np.nan` is coerced to NA (we can see later to add an option / config for this)
- The added tests are for now mostly copied from the existing integer/boolean tests, and adapted for Float
- It directly uses some of the masked ops I implemented earlier for IntegerArray, which means that eg `sum` already shows a similar perf benefit:
  ```python
  a = np.random.randn(1_000_000) 
  a[np.random.randint(0, 1_000_000, 100)] = np.nan 
  s1 = pd.Series(a)
  s2 = pd.Series(a, dtype="Float64")

  In [6]: np.allclose(s1.sum(), s2.sum()) 
  Out[6]: True

  # numpy float series
  In [7]: %timeit s1.sum()   
  2.73 ms ± 104 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

  # nullable float series
  In [8]: %timeit s2.sum()   
  875 µs ± 18.3 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
  ```

- Still to do:
    - [x] Add whatsnew + docs
    - [ ] Division with Int64 dtype can now result in nullable Float Dtype instead of numpy float array

- Other todo's but that can probably be left for follow-up PRs:
    - Update the `convert_dtypes` method to also return nullable floats (one question here: what with floats that are all "integer"-like? I think we want to keep returning nullable int for that, but we might want to add a parameter controlling that behaviour?)
    - Some more arithmetical operations with IntegerArray/BooleanArray can now return a FloatingArray instead of numpy float array
    - Ufuncs on IntegerArray/BooleanArray can return FloatingArray, ufuncs on mixture of IntegerArray/FloatArray
    - Consolidate some tests that are common for integer/boolean/float
    - Deduplicate more things between the Array classes and move to BaseMaskedArray (eg the arithmetic / comparison ops can probably be moved with a bit of refactoring)
        - `__from_arrow__`
        - ufuncs
        - `coerce_to_array`, but this will probably require some refactoring
  - formatting options




Labels:
name: Enhancement
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

Comments:
Hello @jorisvandenbossche! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:































































There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-09-30 08:18:27 UTC
@dsaxton @Dr-Irv thanks for the comments! 
For your comments on `test_arithmetic.py`, that's mostly because I copied those from the integer tests, and it was done like that there as well. Not a reason to improve it of course ;), but I am completely refactoring those arithmetic tests for integer (https://github.com/pandas-dev/pandas/pull/34454), and if that goes through, will do the same here.

And we indeed also need to start having tests where those dtypes are mixed (eg Int64 with Float64)
cc @pandas-dev/pandas-core 
@TomAugspurger @WillAyd regarding sharing code with the other masked arrays (it's not only with integer, but also with boolean): yes, this certainly needs to happen. 

There are plenty of things that should be shared (some will need a bit of refactoring). See also the last todo item in the top post. I have noted some more things to be shared, and I also actually already started deduplicating some things that I noticed while preparing this PR that could easily be done prior to this PR: https://github.com/pandas-dev/pandas/pull/34187, https://github.com/pandas-dev/pandas/pull/34308 (there are also other things that could be done prior, I think). 
(and I also started refactoring the arithmetic tests, with one of the goals to make it easier to share them: https://github.com/pandas-dev/pandas/pull/34454)

But about doing those in *this* PR: I have a slight preference to not do this. First, because it won't actually make the diff of this PR smaller, but rather more complex (as it would involve moving things from `integer.py` to one of the other files). So it won't necessarily make it easier to review this PR. 
And second, because some of those need a bit of refactoring to properly share things. That will certainly be for the better, but it might be cleaner / clearer to do this in separate, more focused PRs (eg rewriting the ufunc machinery, rewriting the `integer/float_arithmetic_ops`, .. I would rather do those in separate PRs).

(and I am also commited to actually doing such follow-ups, like I already started doing some PRs prior to this)

For things that are straight copy-paste and require no changes, those can certainly be done here (or in a pre-cursor PR).
I'm happy to have the deduplication done as followups. That's easier for me
to understand than abstracting things ahead of time.

On Mon, Jun 1, 2020 at 7:23 AM Joris Van den Bossche <
notifications@github.com> wrote:

> @TomAugspurger <https://github.com/TomAugspurger> @WillAyd
> <https://github.com/WillAyd> regarding sharing code with the other masked
> arrays (it's not only with integer, but also with boolean): yes, this
> certainly needs to happen.
>
> There are plenty of things that should be shared (some will need a bit of
> refactoring). See also the last todo item in the top post. I have noted
> some more things to be shared, and I also actually already started
> deduplicating some things that I noticed while preparing this PR that could
> easily be done prior to this PR: #34187
> <https://github.com/pandas-dev/pandas/pull/34187>, #34308
> <https://github.com/pandas-dev/pandas/pull/34308> (there are also other
> things that could be done prior, I think).
> (and I also started refactoring the arithmetic tests, with one of the
> goals to make it easier to share them: #34454
> <https://github.com/pandas-dev/pandas/pull/34454>)
>
> But about doing those in *this* PR: I have a slight preference to not do
> this. First, because it won't actually make the diff of this PR smaller,
> but rather more complex (as it would involve moving things from integer.py
> to one of the other files). So it won't necessarily make it easier to
> review this PR.
> And second, because some of those need a bit of refactoring to properly
> share things. That will certainly be for the better, but it might be
> cleaner / clearer to do this in separate, more focused PRs (eg rewriting
> the ufunc machinery, rewriting the integer/float_arithmetic_ops, .. I
> would rather do those in separate PRs).
>
> (and I am also commited to actually doing such follow-ups, like I already
> started doing some PRs prior to this)
>
> For things that are straight copy-paste and require no changes, those can
> certainly be done here (or in a pre-cursor PR).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/34307#issuecomment-636829484>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOITHDGMYPAFVKC6RBV3RUOMUBANCNFSM4NHWOC7Q>
> .
>

> %timeit s1.sum()   

Out of curiosity, what is driving the improved perf here?
@jbrockmendel it's mostly due to avoiding using the "nan.." versions (eg `nansum` is quite a bit slower as `sum`. See also https://github.com/pandas-dev/pandas/issues/34773).

I think there are mostly two reasons the nanops versions are slower: 1) you need to calculate the mask every time, and 2) the approach nanops takes (copy array + putmask to fill in "neutral" value, eg 0 for sum) seems slower compared to the newer `where` argument in numpy ufuncs. 

Eg for 

```
a = np.random.randn(1_000_000)  
a[np.random.randint(0, 1_000_000, 100)] = np.nan
```

the nansum basically is:

```
In [14]: %%timeit 
    ...: mask = np.isnan(a) 
    ...: temp = a.copy() 
    ...: np.putmask(temp, mask, 0) 
    ...: np.sum(temp) 
2.34 ms ± 48.4 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
```

which is also I suppose more or less what numpy does:

```
In [15]: %timeit np.nansum(a)      
2.15 ms ± 55.9 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
```

whereas with a known mask + where argument:

```
In [16]: %timeit np.sum(a, where=~mask) 
813 µs ± 9.85 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

(now, it also depends on the number of values that are missing how well the different approaches work, I think)

Also, floating-point-wise, both approaches are also not the same, and result in slightly different numbers (`np.nansum` and the multi-line nanops implementation above give exactly the same result, but the `where` approach gives a difference after 14 significant digits)
> needs some performance testing to ensure that didn't break anything on regular float arrays (as there are some changes in the shared code)

The `ensure_float` in the groupby code is the only change in shared code, I think?

The `ensure_float64(ensure_float(values))` compared to `ensure_float64(values)` is certainly slower (since there is is now an added `is_extension_array_dtype` check):

```
In [7]: arr = np.random.randn(1000)  

In [8]: %timeit ensure_float64(arr)    
52 ns ± 0.467 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [9]: %timeit ensure_float64(ensure_float(arr))   
795 ns ± 9.17 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
```

but it's called in a part of the code where before ending up at the line of `ensure_float64(ensure_float(..))`, there have already been checks using `is_numeric_dtype`, `is_integer_dtype`, `is_complex_dtype`, which are all similar in time as the `is_extension_array_dtype` / `ensure_float`, and together take a lot more time.
More feedback on this?

Are people OK with getting this in for 1.1? It won't be able to sit in master for a while, but since it's still experimental anyway and doesn't change any default behaviour (only opt-in), I think that might be fine. 
I am only not sure if I will be able to do the listed follow-ups before 1.1, though.

will look again tom
prob should wait for 1.2 at this late date.
OK, let's indeed postpone this for 1.2. Although experimental anyway (so I don't think it would be a problem to merge now), I won't have time to do some of the needed follow-ups before 1.1.

Review is still welcome though ;) (so this can be merged once 1.1 is tagged)
Can you transplant `FloatBlock.to_native_types` (possibly with a more descriptive name like "to_strings_for_csv") to a FloatArray method.  Then ideally FloatBlock.to_native_types and Float64Index._format_native_types can dispatch here
> Can you transplant FloatBlock.to_native_types (possibly with a more descriptive name like "to_strings_for_csv") to a FloatArray method. Then ideally FloatBlock.to_native_types and Float64Index._format_native_types can dispatch here

Is that related to the introduction of a FloatingArray? (also for the IntegerArray / BooleanArray or other EAs we don't have such a method? ) We can certainly think about where the formatting should live / whether we should add that to the EAs, but I think this PR is already big enough?
> Is that related to the introduction of a FloatingArray?

For floats we of course have actual formatting options, which we don't have for integers/bool .. Still prefer to tackle that separately, though, as that requires some design work how to pass through the options (we already have `EA._formatter` to let the EA control how the values are formatted).
Also, review would be welcome here. I would like to get this merged sooner rather than later, so it can live a while in master.
will look at these prs over the weekend 
> Why just float32 & float64? Does NumPy always have Float16 and Float128 available?

Good question, I suppose I just never really thought about those others .. 
We currently do support storing those in a Series (I don't know if they will be preserved in all operations though, certainly for our own algos, eg groupby is only for 32/64 available at the moment). But I suppose if numpy supports it, we could also (pyarrow has float16/32/64 I think)

> I don't know if they will be preserved in all operations though, certainly for our own algos

I think that's the important piece. If we can't preserve it through our own algos then probably not worth adding it in the first place (at least till someone asks for it). 
Thanks for the reviews! Pushed some updates.

Regarding the comments about things that could/should be shared: I copied a few additional properties that can be shared verbatim, but for all others this will require some refactor, and see my long comment about that above why I think it is better to do that in separate PRs: https://github.com/pandas-dev/pandas/pull/34307#issuecomment-636829484
Any remaining comments here? 
will look again 
thanks @jorisvandenbossche really nice, sorry for the long time to review, as always shorter PRs are *much* easier (but get it!)