DEPR: deprecate SparseArray.values
State: closed

Having a `.values` attribute on SparseArray is confusing, as `.values` is typically used on Series/DataFrame/Index and not on the array classes.



Labels:
name: Deprecate
description: Functionality to remove in pandas

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

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

```diff
@@            Coverage Diff             @@
##           master   #26421      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         174      174              
  Lines       50741    50743       +2     
==========================================
- Hits        46529    46526       -3     
- Misses       4212     4217       +5
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.19% <66.66%> (ø)` | :arrow_up: |
| #single | `41.16% <0%> (-0.18%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26421?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/sparse/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL2ZyYW1lLnB5) | `95.63% <100%> (ø)` | :arrow_up: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.68% <100%> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.6% <100%> (-0.11%)` | :arrow_down: |
| [pandas/core/arrays/sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3NwYXJzZS5weQ==) | `92.71% <40%> (+0.01%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.02% <0%> (-0.12%)` | :arrow_down: |

------

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

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

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

```diff
@@            Coverage Diff             @@
##           master   #26421      +/-   ##
==========================================
+ Coverage   91.74%   91.75%   +<.01%     
==========================================
  Files         174      174              
  Lines       50763    50754       -9     
==========================================
- Hits        46575    46567       -8     
+ Misses       4188     4187       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.26% <70%> (ø)` | :arrow_up: |
| #single | `41.71% <10%> (-0.08%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26421?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/internals/managers.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW50ZXJuYWxzL21hbmFnZXJzLnB5) | `93.93% <ø> (ø)` | :arrow_up: |
| [pandas/core/sparse/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL2ZyYW1lLnB5) | `95.63% <100%> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.7% <100%> (+0.1%)` | :arrow_up: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.68% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3NwYXJzZS5weQ==) | `93.08% <50%> (+0.38%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26421/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.02% <0%> (-0.12%)` | :arrow_down: |

------

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

cc @TomAugspurger since you are most familiar with Sparse nowadays .. (although reluctantly :-))

Removing this here also further entangles a bit the get_values / values mess, as SparseArray is still the only array with .values, and in some places we do `hasattr` or `getattr` on 'values', which then catches SparseArray ..
+1

Looks like a few warnings still https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=11542&view=logs&jobId=521b7dfd-2989-5ff8-bc8c-7481906480fa&taskId=07b8d9d4-6363-5e2d-bc2b-146a30521256&lineStart=154&lineEnd=154&colStart=109&colEnd=115

My other PR is adding

```
filterwarnings =
    error:Sparse:FutureWarning
```

to our setup.cfg. If you make the error message something like SparseArray.values, these warnings would be elevated to errors too (not sure if we want that or not).
Ah, I missed the apply ones. 
It's quite annoying that the output on our CI does not show which tests is causing it ... (due to using xdist). 

There is one (that I actually already knew about, but for now ignored) that is not that easy to solve: the json code (ujson/python/objToJSON.c) checks in C for a 'values' attribute to get the values out of dataframe / series / index etc.
@TomAugspurger @jreback can you have a new look? I added some extra compat code in cython/c code
+1

On Mon, May 20, 2019 at 8:18 AM Joris Van den Bossche <
notifications@github.com> wrote:

> *@jorisvandenbossche* commented on this pull request.
> ------------------------------
>
> In pandas/_libs/reduction.pyx
> <https://github.com/pandas-dev/pandas/pull/26421#discussion_r285587187>:
>
> > @@ -28,6 +28,14 @@ cdef _get_result_array(object obj, Py_ssize_t size, Py_ssize_t cnt):
>      return np.empty(size, dtype='O')
>
>
> +cdef bint _is_sparse_array(object obj):
>
> Yes, exactly for those reasons (It's only used here, and should be removed
> again once we get rid of this deprecation), I would keep it here
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/26421?email_source=notifications&email_token=AAKAOIS6HRAYVGNAUBPWWMDPWKQJZA5CNFSM4HNKAIJ2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZDF2AQ#discussion_r285587187>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIXL3A3WPZHBHQW2H7TPWKQJZANCNFSM4HNKAIJQ>
> .
>

Sorry, there was no urgency at all. Just thought for a moment that the review of Tom was enough, and wanted to get over with this PR. Will wait on your full review then before doing any fixup.
@jorisvandenbossche my main comment was the Is_sparse_array needs to be in util.pyx (doesn't matter that we will eventually remove it), its in the wrong place. 

slight confusion between whether we recommend ``.to_dense()`` or ``np.array()`` for conversions; we should try to be consistent (maybe just deprecate ``.to_dense()``) but another issue (maybe create one).