SparseArray is an ExtensionArray
State: closed

Closes #21978 
Closes #19506 
Closes https://github.com/pandas-dev/pandas/issues/22835

High-level summary: SparseArray is an ExtensionArray. It's no longer an ndarray subclass. The actual data model hasn't changed at all, it's still an array and a `sparse_index`. Only now the sparse values are `self.sparse_values`, rather than `self`.

This isn't really close to being ready yet. I'm going to go through and self-review a bunch of things right now, will call out for others' opinions in specific places.


API discussions:

- [x] Why is the default kind inconsistent between SparseSeries and SparseArray?
    - Possibly get rid of block in a future PR
- [x] Should `.astype(np_dtype)` be sparse or dense?
    - sparse
- [x] What should the inferred type of an empty SparseArray be? `SparseArray([]).dtype`? NumPy defaults to float (`Sparse[float64]`), pandas typically uses object (`Sparse[object]`)
    - sparse
- [x] Policy for warning when converting to dense (e.g. https://github.com/pandas-dev/pandas/pull/22325/files#diff-71caf9627e9687e837e4b1f86ecc6271R390). In the past, pandas would coerce to an ndarray all over the place. Now we at least have a *chance* of knowing when we're doing a bad thing, and warning about it.
    - warn when it's implicit

Labels:
name: API Design
name: Sparse
description: Sparse Data Type
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Hello @TomAugspurger! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/core/arrays/base.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/arrays/base.py) !

 - There are no PEP8 issues in the file [`pandas/core/common.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/common.py) !

 - There are no PEP8 issues in the file [`pandas/core/dtypes/common.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/dtypes/common.py) !

 - There are no PEP8 issues in the file [`pandas/core/dtypes/concat.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/dtypes/concat.py) !

 - There are no PEP8 issues in the file [`pandas/core/dtypes/missing.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/dtypes/missing.py) !

 - There are no PEP8 issues in the file [`pandas/core/internals/__init__.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/internals/__init__.py) !

 - There are no PEP8 issues in the file [`pandas/core/internals/blocks.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/internals/blocks.py) !

 - There are no PEP8 issues in the file [`pandas/core/internals/concat.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/internals/concat.py) !

 - There are no PEP8 issues in the file [`pandas/core/internals/managers.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/internals/managers.py) !

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

 - There are no PEP8 issues in the file [`pandas/core/reshape/reshape.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/reshape/reshape.py) !

 - There are no PEP8 issues in the file [`pandas/core/series.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/series.py) !

 - There are no PEP8 issues in the file [`pandas/core/sparse/api.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/sparse/api.py) !

 - There are no PEP8 issues in the file [`pandas/core/sparse/array.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/sparse/array.py) !

 - There are no PEP8 issues in the file [`pandas/core/sparse/dtype.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/sparse/dtype.py) !

 - There are no PEP8 issues in the file [`pandas/core/sparse/frame.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/sparse/frame.py) !

 - There are no PEP8 issues in the file [`pandas/core/sparse/series.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/core/sparse/series.py) !

 - There are no PEP8 issues in the file [`pandas/tests/api/test_api.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/api/test_api.py) !

 - There are no PEP8 issues in the file [`pandas/tests/dtypes/test_common.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/dtypes/test_common.py) !

 - There are no PEP8 issues in the file [`pandas/tests/dtypes/test_dtypes.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/dtypes/test_dtypes.py) !

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

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

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

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

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

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

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_api.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/frame/test_api.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_indexing.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/frame/test_indexing.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_subclass.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/frame/test_subclass.py) !

 - There are no PEP8 issues in the file [`pandas/tests/internals/test_internals.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/internals/test_internals.py) !

 - There are no PEP8 issues in the file [`pandas/tests/reshape/test_reshape.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/reshape/test_reshape.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_combine_concat.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/series/test_combine_concat.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_missing.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/series/test_missing.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_quantile.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/series/test_quantile.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_subclass.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/series/test_subclass.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/frame/test_apply.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/frame/test_apply.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/frame/test_frame.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/frame/test_frame.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/frame/test_to_from_scipy.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/frame/test_to_from_scipy.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/series/test_series.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/series/test_series.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_arithmetics.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_arithmetics.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_array.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_array.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_combine_concat.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_combine_concat.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_dtype.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_dtype.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_format.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_format.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_groupby.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_groupby.py) !

 - There are no PEP8 issues in the file [`pandas/tests/sparse/test_indexing.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/tests/sparse/test_indexing.py) !

 - There are no PEP8 issues in the file [`pandas/util/_test_decorators.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/util/_test_decorators.py) !

 - There are no PEP8 issues in the file [`pandas/util/testing.py`](https://github.com/TomAugspurger/pandas/blob/37a4b576b3f4978e797e75e2e0745127a8960d57/pandas/util/testing.py) !

 - Complete extra results for this file :

> file_to_check.py:2518:-258: W605 invalid escape sequence '\('
file_to_check.py:2518:-255: W605 invalid escape sequence '\)'
---



##### Comment last updated on October 12, 2018 at 17:58 Hours UTC
+1; it looks like a bunch of `_libs.util` machinery for `__getitem__` and `__setitem__` are only relevant for `SparseArray`.  When that no longer subclasses `np.ndarray`, a bunch of simplification will be on the table.
Does anyone have thoughts on changing the default SparseArray fill value to match the default for SparseSeries? It's jarring that they aren't the same. Which is more useful in practice, integer or block?
I would personally even say (unless some users stand up): get rid of block altogether. 
(but so, +1 on at least making it consistent)
Regarding what you said on Twitter :-), would it make sense to actually use pydata/sparse here for the core pandas implementation of SparseArray instead of our own custom implementation?
+1 on making consistent
this is going to have a big note in 0.24 anyhow so breaking changes is ok

also +1 on using pydata/sparse as well
though should prob do as a follow up (though maybe if it’s easier do here)
In another issue @jorisvandenbossche and I discussed moving fill_value to the dtype. This leads to behavior like this following:

```python
In [10]: arr = pd.SparseArray([np.nan, 1, 2])

In [11]: arr.astype('Sparse[int]')
Out[11]:
[0, 1, 2]
Fill: 0
IntIndex
Indices: array([1, 2], dtype=int32)
```

This mean `asarray(arr.astype(dtype))` won't necessary be the same as `asarray(arr).astype(dense_dtype)`.  How do people feel about that?
Yes, at least it does locally.

On Sat, Aug 18, 2018 at 2:33 PM jbrockmendel <notifications@github.com>
wrote:

> *@jbrockmendel* commented on this pull request.
> ------------------------------
>
> In pandas/_libs/reshape.pyx
> <https://github.com/pandas-dev/pandas/pull/22325#discussion_r211084211>:
>
> > @@ -1,4 +1,4 @@
> -# cython: profile=False
> +# -*- coding: utf-8 -*-
>
> Will rebasing get this out of the diff?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22325#pullrequestreview-147439436>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIjqnOR3W56RAe7aJ7SOmlNyygUsnks5uSGv6gaJpZM4V7I12>
> .
>

I'm :+1: for coercing `fill_value`. It sounds intuitively reasonable whereas `np.array(np.nan).astype(int) == np.array(-9223372036854775808)` is a mere implementation detail.

I'm also :+1: for `.astype(dtype)` to _retain sparsity_. Or will blocks delegate to re-sparsify sparse series when `df.astype()` is called? All else being equal, `df.astype(int)` resulting in all dense blocks, `df.astype('Sparse[int]')` resulting in all sparse blocks, and manually subselecting columns before type-converting them all look hassle.
> I'm also 👍 for .astype(dtype) to retain sparsity. Or will blocks delegate to re-sparsify sparse series when df.astype() is called?

Right now `Series.astype(numpy_dtype)` and `DataFrame.astype(numpy_dtype)` will maintain sparsity. I'll probably change `SparseArray.astype()`  to do the same. If you want to astype to a dense array, it'll have to be `SparseArray.to_dense().astype(numpy_dtype)`.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22325?src=pr&el=h1) Report
> Merging [#22325](https://codecov.io/gh/pandas-dev/pandas/pull/22325?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/12a0dc49ac63b63bcc5f93bccf71bce51c60bdad?src=pr&el=desc) will **decrease** coverage by `0.06%`.
> The diff coverage is `94.14%`.

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

```diff
@@            Coverage Diff             @@
##           master   #22325      +/-   ##
==========================================
- Coverage    92.2%   92.13%   -0.07%     
==========================================
  Files         169      170       +1     
  Lines       50888    51073     +185     
==========================================
+ Hits        46919    47056     +137     
- Misses       3969     4017      +48
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.56% <94.14%> (-0.07%)` | :arrow_down: |
| #single | `42.28% <42.21%> (+0.04%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22325?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/\_test\_decorators.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX3Rlc3RfZGVjb3JhdG9ycy5weQ==) | `92.68% <ø> (ø)` | :arrow_up: |
| [pandas/core/internals/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW50ZXJuYWxzL19faW5pdF9fLnB5) | `100% <ø> (ø)` | :arrow_up: |
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `95.94% <ø> (ø)` | :arrow_up: |
| [pandas/core/dtypes/missing.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL21pc3NpbmcucHk=) | `92.98% <ø> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `86.42% <100%> (+0.23%)` | :arrow_up: |
| [pandas/core/reshape/reshape.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9yZXNoYXBlLnB5) | `99.55% <100%> (ø)` | :arrow_up: |
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `95.11% <100%> (+0.07%)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.86% <100%> (+0.1%)` | :arrow_up: |
| [pandas/core/sparse/dtype.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL2R0eXBlLnB5) | `100% <100%> (ø)` | |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.39% <100%> (-2.8%)` | :arrow_down: |
| ... and [30 more](https://codecov.io/gh/pandas-dev/pandas/pull/22325/diff?src=pr&el=tree-more) | |

------

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

Quick update on `SparseArray.astype`:

- for `np.dtype` like `arr.astype(int)` the fill value will *not* be changed. This was causing too many hacks inside pandas itself, and was too surprising to the user.
- For SparseDtype like `arr.astype(SparseDtype(int, 0)` the fill value will be updated.

See the docstring for more.
FYI, if I haven't stated it explicitly, my current hope is to create a `.sparse` accessor and deprecate SparseSeries and SparseDataFrame in favor of Series / DataFrame holding spares values. IIRC, last we looked the major differences were that SparseDataFrame.__setitem__ behaved differently, as newly created columns are sparsified. We'll see if anything else comes up.
This should be getting close. Definitely at the stage where reviews would be appreciated. I tried to indicate behavior changes with an `XXX` in the source.

I'm going to go through and remove / fix all remaining xfails that I added.

The last major item is ufuncs... With the lovely new `__array_ufunc__`, this was pretty easy to do, but that's only available from NumPy 1.13. I see two options:

1. Implement `__array_wrap__`, `__array_prepare__`, and `__array_priority__`. The [ufunc NEP](https://www.numpy.org/neps/nep-0013-ufunc-overrides.html) mentions that
> Trying to ufunc things that don’t subclass ndarray is even more difficult, as the input arguments tend to be cast to object arrays, which ends up producing surprising results.
so we can see how far that gets us.
2. Check the NumPy version when creating a SparseArray and raise a RuntimeError (or ImportError) if it's too old.

neither sound great, but out of laziness I'm currently leaning towards 2. If someone cares about Sparse + NumPy < 1.13, I won't object to a PR implementing it.
All the new xfails / skips should be accounted for. Removed the "WIP" tag

Answering some of the questions from the original:

Q. Why is the default kind inconsistent between SparseSeries and SparseArray?
A. Who knows, but I'd like to get rid for block in a future PR.

Q. Should .astype(np_dtype) be sparse or dense?
A. sparse

Q. What should the inferred type of an empty SparseArray be?
A. Sparse[float]

Q. Policy for warning when converting to dense (e.g. https://github.com/pandas-dev/pandas/pull/22325/files#diff-71caf9627e9687e837e4b1f86ecc6271R390).
A. Warn when we have to implicitly densify as part of an operation.
@TomAugspurger thanks a lot for your work on this, this seems a gigantic task

In general, I am wondering a bit, since we are already breaking things and changing behaviour, if we shouldn't be more aggressive in deprecating / removing certain parts of the API. For example the sparse_index concept, the int vs block type. 
Reasons that I am raising this is a) this API seems rather specific (to pandas?) and do users actually use/need this? (by now refactoring it and keepin it, it feels like committing more strongly to this API, while until now I would personally not have considered/recommended the sparse functionality as a core or very stable part of pandas) and b) this might make it possible to actually swap the underlying implementation to another sparse array implementation (at an undefined point in the future), eg pydata/sparse.



I don't think it's that strange, and it was allowed / tested before.

Previously, you couldn't do

```python
arr = SparseArray([1, 2], fill_value=0)
arr.fill_value = np.nan
```

but you could do

```python
arr = SparseArray([1, 2], fill_value=np.nan)
```


I should move this to the bug fixes section.

On Thu, Aug 23, 2018 at 2:45 PM Joris Van den Bossche <
notifications@github.com> wrote:

> *@jorisvandenbossche* commented on this pull request.
> ------------------------------
>
> In doc/source/whatsnew/v0.24.0.txt
> <https://github.com/pandas-dev/pandas/pull/22325#discussion_r212434126>:
>
> > +
> +``SparseArray`` is now an ``ExtensionArray``
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +SparseArray is now implements the ExtensionArray interface.
> +To conform to this interface, and for consistency with the rest of pandas, some API breaking
> +changes were made:
> +
> +- ``SparseArray`` is no longer a subclass of :class:`numpy.ndarray`
> +- ``SparseArray.dtype`` and ``SparseSeries.dtype`` are now instances of ``SparseDtype``, rather than ``np.dtype``. Access the underlying dtype with ``SparseDtype.subtype``.
> +- :meth:`numpy.asarray(sparse_array)` now returns a dense array with all the values, not just the non-fill-value values (:issue:`14167`)
> +- Passing a ``fill_value`` to ``SparseArray.take`` no longer implies ``allow_fill=True``.
> +- ``SparseArray.take`` no longer accepts scalars for indices.
> +- ``SparseArray.astype(sparse_dtype)`` will now change both the dtype of the underlying ``sp_values`` and the ``fill_value``. Previously, just
> +  ``sparse_array.sp_values.dtype`` was changed. The same holds for a Series with spares values.
> +- Setting ``SparseArray.fill_value`` to a fill value with a different dtype is now allowed.
>
> It seems a bit strange though to have sp_values and a fill_value that
> don't have compatible dtypes?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22325#discussion_r212434126>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIhXq9oBNqxZk6vmUh3mJJnlzHb89ks5uTwZjgaJpZM4V7I12>
> .
>

But you could also say that it is the second example that was 'buggy' ? Because although that was allowed, the sparse array actually has no occurrences of this fill value.

One possible problem I see with this, is that then `arr.to_dense()` will not always be of the same dtype as `arr`, which I think is what the expected behaviour should be.
There is in any case still something buggy here (with the latest version of your branch):

```
In [1]: arr = pd.SparseArray([0, 1, 0], fill_value=0)

In [2]: arr.to_dense()
Out[2]: array([0, 1, 0])

In [3]: arr.fill_value = np.nan
/home/joris/miniconda3/envs/dev/bin/ipython:1: PerformanceWarning: Updating fill_value requires converting to a dense array
  #!/home/joris/miniconda3/envs/dev/bin/python

In [4]: arr
Out[4]: 
[0, 1, 0]
Fill: nan
IntIndex
Indices: array([0, 1, 2], dtype=int32)

In [5]: arr.to_dense()
Out[5]: array([0, 1, 0])
```

The dense version still has the 0's and not the new fill value NaN
Ah, sorry, that is because now the 0's have become actual values... 
OK, that is a bit different behaviour as in sparse. I expected that the fill_value would have been *replaced*, but actually the performance warning was a good indication
@TomAugspurger this seems to be a (rather big) break in behaviour. On 0.23, setting the fill_value *replaces* the fill values:

```
In [3]: arr = pd.SparseArray(np.array([0, 1, 2, 3, 0]))

In [4]: arr.fill_value = 10

In [5]: arr
Out[5]: 
[10, 1, 2, 3, 10]
Fill: 10
IntIndex
Indices: array([1, 2, 3], dtype=int32)
```

while with this branch:

```
In [40]: arr = pd.SparseArray(np.array([0, 1, 2, 3, 0]))

In [41]: arr.fill_value = 10
/home/joris/miniconda3/envs/dev/bin/ipython:1: PerformanceWarning: Updating fill_value requires converting to a dense array
  #!/home/joris/miniconda3/envs/dev/bin/python

In [42]: arr
Out[42]: 
[0, 1, 2, 3, 0]
Fill: 10
IntIndex
Indices: array([0, 1, 2, 3, 4], dtype=int32)
```

The behaviour of 0.23 is also what eg pydata/sparse does (so my "wrong" expectation from above was not that much off :-)). So I suppose this was an unintended change? 
I would also think it should simplify the implementation somewhat.
What do you expect here?

```python
arr = SparseArray([0, 1, 2])
arr.fill_value = 1
arr.fill_value = 0
```

a.) [0, 1, 2]
b.) [0, 0, 2]

This whole setting business is strange. 

The motivation for the PerformanceWarning / coercing to array is so that we can ensure that `fill_value` is not present in the `sp_values`.
I expect a). I think in general the fill_value and actual values should always be handled separately, and the sparse structure should not change during operations

> so that we can ensure that fill_value is not present in the sp_values.

I don't think it is needed to ensure that.

I have to agree that is one of the things I always found confusing with sparse matrices in scipy, but there this is not ensured. So a typical sparse matrix in scipy can have two 'kinds' of 0, the fill_value and 0 in the data.
> I expect a).

In any case that is what it currently does on master, and it is what pydata/sparse does
Alrightly, that's been implemented (and did indeed simplify the setter). I added a note that you can have the two kinds of `fill value` values.

```python
In [6]: arr = pd.SparseArray([0, 1, 2], fill_value=0)

In [7]: arr.fill_value = 1

In [8]: arr
Out[8]:
[1, 1, 2]
Fill: 1
IntIndex
Indices: array([1, 2], dtype=int32)

In [9]: arr.fill_value = 0

In [10]: arr
Out[10]:
[0, 1, 2]
Fill: 0
IntIndex
Indices: array([1, 2], dtype=int32)
```
Latest commit removes inplace ops between ndarray and SparseArray. I wasn't doing it properly in __array_ufunc__ earlier.

With some work they could probably be made to work, but they don't work on master either.
Ah, I was misremembering https://github.com/pandas-dev/pandas/issues/19750,
that is not resolved.

On Tue, Sep 4, 2018 at 7:31 AM Jeff Reback <notifications@github.com> wrote:

> *@jreback* commented on this pull request.
> ------------------------------
>
> In doc/source/whatsnew/v0.24.0.txt
> <https://github.com/pandas-dev/pandas/pull/22325#discussion_r214894955>:
>
> > +.. _whatsnew_0240.api_breaking.sparse_values:
> +
> +``SparseArray`` is now an ``ExtensionArray``
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +SparseArray now implements the ExtensionArray interface.
> +To conform to this interface, and for consistency with the rest of pandas, some API breaking
> +changes were made:
> +
> +- ``SparseArray`` is no longer a subclass of :class:`numpy.ndarray`. To convert a SparseArray to a NumPy array, use :meth:`numpy.asarray`.
> +- ``SparseArray.dtype`` and ``SparseSeries.dtype`` are now instances of ``SparseDtype``, rather than ``np.dtype``. Access the underlying dtype with ``SparseDtype.subtype``.
> +- :meth:`numpy.asarray(sparse_array)` now returns a dense array with all the values, not just the non-fill-value values (:issue:`14167`)
> +- ``SparseArray.take`` now matches the API of :meth:`pandas.api.extensions.ExtensionArray.take`.
> +  * The default value of ``allow_fill`` has changed from ``False`` to ``True``.
> +  * The ``out`` and ``mode`` parameters are now longer accepted (previously, this raised if they were specified).
> +  * Passing a scalar for ``indices`` is no longer allowed.
>
> no as far as I know. That has always been the standard in whatsnew notes.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22325#discussion_r214894955>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIm0Ltl3NhvPhPzFqIWkQjS-aP6_1ks5uXnKtgaJpZM4V7I12>
> .
>

Just pushed an update. The biggest change is that the extension array tests are now parametrized by `fill_value` (previously it was just nan). Some of the base tests have to be skipped because we can't build the `expected` properly.

- Some rely on being able to do `SparseArray([stuff, np.nan], dtype=dtype)`, which fails for int dtype (can't have nan in an int)
- Some rely on `op(self, other)` being equivalent to `self.combine(other)`. This fails because SparseArray ops change the `fill_value` to be `op(self.fill_value, other)`, to maintain the sparsity. Combine can't do that, so we can't make the expected.

For now I just skip these cases, since they are tested elsewhere.
How do we want to proceed here? I'd like to let this rest in master for a while. I also need to get started on refactoring Period (which will hopefully be smaller).

Is there anything I can do to make reviewing this easier? There are a couple more incidental change I could split off (`is_bool_dtype/indexer`, cross section of homogenous frame... that's about it). I think that removing BlockIndex in this PR would make reviewing harder, though I may be wrong.




will look in a day or 2

if u what to split what u described above by me