DEPR: Deprecate Series/Dataframe.to_dense/to_sparse
State: closed

- [x] closes  #26557 
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] add whatsnew entry
- [x] add tests

Marks the methods mentioned in #26557 as deprecated, with proper messages.

Labels:
name: Sparse
description: Sparse Data Type
name: Deprecate
description: Functionality to remove in pandas

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26684?src=pr&el=h1) Report
> Merging [#26684](https://codecov.io/gh/pandas-dev/pandas/pull/26684?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/891a419a5155e6b42c0696a81cf853b6f3febbf7?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #26684      +/-   ##
==========================================
+ Coverage   91.87%    91.9%   +0.02%     
==========================================
  Files         174      174              
  Lines       50661    50916     +255     
==========================================
+ Hits        46547    46796     +249     
- Misses       4114     4120       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.45% <100%> (+0.04%)` | :arrow_up: |
| #single | `41.79% <36.36%> (-0.08%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26684?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/sparse/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL2ZyYW1lLnB5) | `96.59% <100%> (+0.96%)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97% <100%> (-0.12%)` | :arrow_down: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.62% <100%> (ø)` | :arrow_up: |
| [pandas/core/sparse/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3Nlcmllcy5weQ==) | `95.75% <100%> (+0.85%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `96.62% <0%> (+0.34%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26684?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/26684?src=pr&el=footer). Last update [891a419...39230d4](https://codecov.io/gh/pandas-dev/pandas/pull/26684?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/26684?src=pr&el=h1) Report
> Merging [#26684](https://codecov.io/gh/pandas-dev/pandas/pull/26684?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/baa77c33fb71c29acea21ba06adaf426ed4cb561?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/26684/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26684?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26684      +/-   ##
==========================================
- Coverage   91.87%   91.86%   -0.01%     
==========================================
  Files         180      180              
  Lines       50712    50719       +7     
==========================================
+ Hits        46590    46593       +3     
- Misses       4122     4126       +4
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.46% <100%> (ø)` | :arrow_up: |
| #single | `41.11% <66.66%> (-0.08%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26684?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/groupby/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9vcHMucHk=) | `96% <100%> (ø)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.89% <100%> (-0.12%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `94.2% <100%> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.66% <100%> (+0.01%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26684/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `88.88% <0%> (-11.12%)` | :arrow_down: |

------

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

@jorisvandenbossche Thanks for the review. I committed the stylistic changes that you suggested. 

Working on writing the tests. Could you please guide me on where the tests for Series and Dataframe (not sparse) need to be written? Should I create a new file or write it in an existing file. 
Hello @intEll1gent! 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 2019-06-18 04:51:35 UTC
>   ...
> * There is an additional `to_dense` method in generic.py that should also be deprecated.
> ...

@jorisvandenbossche 
- Deprecating to_dense in `generic.py` breaks some tests for other classes. (I still haven't pushed that part). I could modify those tests but can't figure out how to ignore the FutureWarning. `@pytest.mark.filterwarnings('ignore::FutureWarning')` doesn't seem to help. 
- The other deprecations that I did raise a lot of warnings in many of the tests (but they pass)

Also, I'll change the code style issues in the next push. 
@intEll1gent I would need to look myself to say for sure (but now away for the weekend),  but if other tests raise those warnings, then it might be because the methods are still used in certain places in pandas. In such a case, you will need to check those places and see if it can be replaced with something else (or if it was just used in the tests directly, then you can add a filterwarnings decorator).
> @intEll1gent I would need to look myself to say for sure (but now away for the weekend), but if other tests raise those warnings, then it might be because the methods are still used in certain places in pandas. In such a case, you will need to check those places and see if it can be replaced with something else (or if it was just used in the tests directly, then you can add a filterwarnings decorator).

Turns out changing `UserWarning` to `Warning, check_stacklevel=False` lets them pass. 

> again, I don't want blanket warning filters at all. These are hiding errors.

For the old sparse tests (so not speaking about the groupby tests, there we indeed should certainly not hide the warnings), we decided earlier to not rewrite those tests but just ignore the warnings (until all functionality + tests can be removed). So you will see there are already existing filterwarnings for Sparse. For those cases (again, tests about the *old, deprecated* SparseDataFrame/Series, I think it is best to also ignore those new warnings).
> we still have to come to an unanimous decision whether to keep the double deprecation warning for Series/DataFrame.to_sparse. None of this will be needed if we deicide to stick with only 1 warning for the main deprecation.

@intEll1gent I think it is fine to have the double deprecation. How would you propose to only have a single warning? I personally think we certainly need a warning that mentions `to_sparse`, so that would mean suppressing the other warning from inside `to_sparse`?
> > again, I don't want blanket warning filters at all. These are hiding errors.
> 
> For the old sparse tests (so not speaking about the groupby tests, there we indeed should certainly not hide the warnings), we decided earlier to not rewrite those tests but just ignore the warnings (until all functionality + tests can be removed). So you will see there are already existing filterwarnings for Sparse. For those cases (again, tests about the _old, deprecated_ SparseDataFrame/Series, I think it is best to also ignore those new warnings).

right this case is ok, but we need to be really clear on this, e.g. on ``pandas/tests/sparse`` should have these types of filters
> Not added by me. Also present on upstream.

@intEll1gent for future reference: you are correct that it was not added by you, but in general, the diff that github shows on the PR needs to be correct. So if you noticed unrelated changes in the diff here from master, that means that something went wrong when you updated this branch to the latest master. And it is still up to you to fix that. See my earlier comment on how that could be done.
> right this case is ok, but we need to be really clear on this, e.g. on pandas/tests/sparse should have these types of filters

@jreback yes, that sounds correct, that we should in principle only do this general filterwarnings in the `pandas/tests/sparse`. However, it seems that with the original SparseDataFrame deprecation, such a general filterwarning was added in several other places as well (@TomAugspurger ? eg the `pandas/tests/arrays/sparse/test_arithmetic.py` do you remember why the general filterwarnings on Sparse was needed there?)
> > right this case is ok, but we need to be really clear on this, e.g. on pandas/tests/sparse should have these types of filters
> 
> @jreback yes, that sounds correct, that we should in principle only do this general filterwarnings in the `pandas/tests/sparse`. However, it seems that with the original SparseDataFrame deprecation, such a general filterwarning was added in several other places as well (@TomAugspurger ? eg the `pandas/tests/arrays/sparse/test_arithmetic.py` do you remember why the general filterwarnings on Sparse was needed there?)

right, let's comment these warning to be clear where they come from if they are not in pandas/tests/sparse (and actually maybe there too)


> > we still have to come to an unanimous decision whether to keep the double deprecation warning for Series/DataFrame.to_sparse. None of this will be needed if we deicide to stick with only 1 warning for the main deprecation.
> 
> @intEll1gent I think it is fine to have the double deprecation. How would you propose to only have a single warning? I personally think we certainly need a warning that mentions `to_sparse`, so that would mean suppressing the other warning from inside `to_sparse`?

Yes, I changed `to_sparse` to ignore the SDF/SS warnings and only show a single warning. The main warning shows in other cases related to SDF/SS. 
i will review at some point again
thanks @intEll1gent !