Remove SparseSeries and SparseDataFrame #28425

TomAugspurger · 2019-09-13T03:22:18Z

As discussed on the dev call.

Closes #28407
Closes #28025
Closes #26777
Closes #26227
~~Not #26123~~
Closes #25694
Closes #25378
~~Not #25270~~
~~Not #24817~~
Closes #24270
Closes #22630
Closes #21993
Closes #21909
~~Not #21818~~
Closes #21231
Closes #20766
Closes #20172
~~Not #19278~~
Closes #18969
~~Not #18414~~
Closes #17993
Closes #15737
Closes #15634
Closes #14427
Closes #14310
Closes #13665
~~Not #11726~~
Closes #10627
Closes #9275
~~Not #9265~~
Closes #6427
Closes #5470
Closes #5078

jorisvandenbossche

You can do a search for @pytest.mark.filterwarnings("ignore:Sparse:FutureWarning"), I thnk there are a few left (or at least at the moment I was reviewing)

doc/source/whatsnew/v1.0.0.rst

pandas/core/ops/__init__.py

pandas/core/reshape/concat.py

jorisvandenbossche · 2019-09-13T11:59:10Z

pandas/core/sparse/scipy_sparse.py

@@ -1,7 +1,7 @@
 """
 Interaction with scipy.sparse matrices.

-Currently only includes SparseSeries.to_coo helpers.


I would move this file to arrays/sparse.py (or as separate file next to it), since it is only used there

Yeah, I was going to do moves as a followup. I'd also like to move the last bit of pandas/tests/sparse/test_combine_compat.py, which had a SparseArray test in it.

pandas/core/sparse/api.py

pandas/io/pytables.py

jorisvandenbossche · 2019-09-13T12:03:47Z

pandas/tests/io/generate_legacy_storage_files.py

@@ -53,10 +53,7 @@
    Period,
    RangeIndex,
    Series,
-    SparseDataFrame,


I think you can leave this file intact for now?
(isn't it meant to be run with older version of pandas? In case we need to regenerate old pickle files, we might still want to generate sparse ones to check that reading them works / errors with a good message)

IIUC, we need to remove it so that the sparse structures are not included, re-run it with an older pandas, and commit the new binary file.

I will update to use SparseArray though.

@WillAyd can you comment here? I think we import this file when running the tests, so we need to be able to import it under both the old and new pandas?

Should this file now be restored now you added the pickle compat? (I also think you now no longer update the old pickle files, right?)

I think this file gets imported on master, so I need to remove this.

Right the file would be created with an old release but read and tested against master I think

So if we want to mimic what users can do and what Tom is trying to support (i.e. write a pickle file with SparseDataFrame in an older release and read this file with the new release), this file needs to be able to generate such pickle files with SparseDataFrame?

Now, I don't really care that much about SparseDataFrame, and there are currently already legacy pickle files that contain it, so the need to be able to recreate those files before we will actually remove that support is maybe not that high, and I am maybe splitting hairs.

Sorry @jorisvandenbossche, I think I've misunderstood you this whole time (and I was confused why you kept pushing on this point 😄). You're correct that if we relied on these legacy files for testing reading files written with SparseSeries & SparseDataFrame, then we'd need to keep the imports here (at conditionally).

But a8b0d65 added tests dedicated for that (we need dedicated tests, since the existing ones silence all warnings).

So from a test coverage POV, we're OK with removing writing SparseSeries & SparseDataFrame here. It's just a matter of code style / prep for running this file when 1.0.0 becomes the "legacy" pickle version we support.

So from a test coverage POV, we're OK with removing writing SparseSeries & SparseDataFrame here.

Yep, if we have files now, it seems ok to remove it from this file. It seems rather unlikely that after the next bump in legacy support for pickle files, we still want to keep support for SparseDataFrame, I suppose, as we now warn for it?

That sounds right to me. Technically, we could chose to bump the next legacy to version 0.25.x, which has SparseDataFrame, but I think we're more likely to choose 1.0.x since it's a round number.

pandas/tests/io/json/test_pandas.py

pandas/tests/series/test_combine_concat.py

TomAugspurger · 2019-09-13T15:37:32Z

@WillAyd it looks like you fixed up the pickle panel stuff in https://github.com/pandas-dev/pandas/pull/27082/files.

Would you recommend removing SparseSeries / SparseDataFrame from the pickle files as a precursor to this PR?

TomAugspurger · 2019-09-13T15:58:14Z

3cc4765 has the changes to the pickle stuff. I can split that out into a precursor PR if people want.

I regenerated the pickle files with python 3.5.6 and pandas 0.20.3.

jorisvandenbossche · 2019-09-13T16:05:35Z

Do we still have pickle files with sparse data in them?
What error do you get if you read such a file with this branch?

TomAugspurger · 2019-09-13T16:09:25Z

Do we still have pickle files with sparse data in them?
What error do you get if you read such a file with this branch?

As of 3cc4765 we have a pickle file with Series[Sparse] and DataFrame[Sparse], but no SparseSeries and SparseDataFrame. That converted the use of SparseSeries/Frame to Series/Frame with sparse values.

With 3cc4765, the pickle tests on this branch pass. It would also pass on master, so I could split those changes out.

jorisvandenbossche · 2019-09-13T16:15:16Z

I don't have a strong opinion on splitting or not, either way.

For older pickle files with SparseDataFrame in it, you currently get:

AttributeError: Can't get attribute 'SparseDataFrame' on <module 'pandas.core.sparse.frame' from '/home/joris/scipy/pandas/pandas/core/sparse/frame.py'>

With our pickle_compat, I suppose we could actually map this to a function that creates a DataFrame[sparse] of it. But not sure that's worth the effort.
But a better error message indicating they need to read it in 0.25, convert and save again to read it in 1.0 would be welcome I think.

TomAugspurger · 2019-09-13T17:08:31Z

Huh, 766a2f2 has a bit of compat code to allow reading a file saved with SparseSeries / SparseFrame. I don't really know how it works, but it seems to. I've restored the old pickle file with SparseSeries / SparseFrame and it passes locally.

jorisvandenbossche

This looks good to me.

For the docs, we need to see if it is needed to add actual output to the code-blocks

For the issues you are closing with this, we actually need to see if some of them are not also valid for Series[sparse] / SparseArray

jorisvandenbossche · 2019-09-16T18:44:21Z

pandas/tests/io/generate_legacy_storage_files.py

@@ -53,10 +53,7 @@
    Period,
    RangeIndex,
    Series,
-    SparseDataFrame,


Should this file now be restored now you added the pickle compat? (I also think you now no longer update the old pickle files, right?)

jorisvandenbossche · 2019-09-16T19:19:18Z

For the issues you are closing with this, we actually need to see if some of them are not also valid for Series[sparse] / SparseArray

I already went through the most recent ones, and crossed a few in the top post.

WillAyd · 2019-09-16T23:38:08Z

pandas/tests/io/generate_legacy_storage_files.py

@@ -53,10 +53,7 @@
    Period,
    RangeIndex,
    Series,
-    SparseDataFrame,


The changes here are correct; you just need to re-run this in an environment with pandas 0.20.3 ideally using the minimum supported Python version, which I guess right now is 3.5.3 though by the time this goes live should be 3.6, so maybe stick to the latter (I see there is one for 3.5.6 already)

pandas/tests/io/generate_legacy_storage_files.py

WillAyd

Generally nice! This is quite the clean up. Just some comments to consider but I think this looks really good

doc/source/user_guide/io.rst

WillAyd · 2019-09-16T23:49:52Z

doc/source/whatsnew/v0.16.0.rst

@@ -91,8 +91,7 @@ Interaction with scipy.sparse

 Added :meth:`SparseSeries.to_coo` and :meth:`SparseSeries.from_coo` methods (:issue:`8048`) for converting to and from ``scipy.sparse.coo_matrix`` instances (see :ref:`here <sparse.scipysparse>`). For example, given a SparseSeries with MultiIndex we can convert to a `scipy.sparse.coo_matrix` by specifying the row and column labels as index levels:

-.. ipython:: python
-   :okwarning:
+.. code-block:: python


Could be done as a follow up and probably something easy for the community to contribute to, but ideally since we are converting these from ipython to python directives should add the original output (applicable in a few places below)

Certainly wouldn't object, but I think that's fairly low-value since, e.g. the 0.16 version of the docs at https://pandas.pydata.org/pandas-docs/version/0.16.0/whatsnew.html will have the correct rendering.

Yea I don’t think you should do here as there’s already enough - just a nice to have a first time contributor could pick up.

I can open an issue for it later

pandas/compat/pickle_compat.py

jorisvandenbossche · 2019-09-17T07:58:11Z

added new legacy pickle files

Maybe rather add files for a different (later) version, since we already have them for 0.21?

TomAugspurger · 2019-09-17T18:59:14Z

Just FYI, since the last round of reviews I've added

A dummy SparseDataFrame and SparseSeries class, mirroring what we did for Panel. This prevents the immediate error with feather (I've removed the test xfails and reverted the doc changes working around it)
Added a warning when loading a pickle file with a SparseDataFrame / SparseSeries saying they should re-save it

jorisvandenbossche

Last changes look good to me!

TomAugspurger · 2019-09-17T20:57:20Z

Going through the rest of the issues to validate that they are specific to the sparse subclasses.

Edit: done. The list at the top should be accurate. All the remaining sparse issues still open should be relevant to SparseArray.

jorisvandenbossche · 2019-09-18T11:27:22Z

Let's merge this?

jreback · 2019-09-18T11:51:03Z

lgtm. I am -0 on providing the pickle compat. but I guess ok.

jreback · 2019-09-18T11:51:18Z

merge when ready.

TomAugspurger · 2019-09-18T14:01:41Z

Gave another glance through, and I think we're good.

Mostly agreed about the pickle compat. But since it turned out to be so easy (same internal representation) I think we're OK.

jorisvandenbossche · 2019-09-18T14:06:26Z

Woohoo! :)

I am -0 on providing the pickle compat. but I guess ok.

I think a reason to have it (apart from that it was easy to add) is because this is a very recent deprecation, and that it also maps nicely to a supported data structure.

TomAugspurger added 4 commits September 12, 2019 15:57

CLN: Remove sparse

acf5f2f

round 2

5418dd5

Round 3

f61b5e3

round 4

238db69

TomAugspurger added this to the 1.0 milestone Sep 13, 2019

TomAugspurger added 4 commits September 13, 2019 06:47

remove hdf

7448795

some more

f285272

cleanup

b6fb1aa

note

c476b21

jorisvandenbossche reviewed Sep 13, 2019

View reviewed changes

TomAugspurger added 2 commits September 13, 2019 10:56

fixups

fc34fe8

pickle changes

3cc4765

TomAugspurger added 4 commits September 13, 2019 11:57

pickle compat

766a2f2

skip feather

dd51140

fixups

129e89e

Merge remote-tracking branch 'upstream/master' into remove-sparse

5b711c6

TomAugspurger added 3 commits September 13, 2019 13:25

cleanups

075bfd2

black

2b58e53

to_sparse docs

413347f

TomAugspurger added Deprecate Functionality to remove in pandas Sparse Sparse Data Type labels Sep 13, 2019

TomAugspurger added 3 commits September 13, 2019 14:11

doc note

d5828e3

rm sparse frame

047773e

rm sparse series

2d8d195

TomAugspurger mentioned this pull request Sep 16, 2019

REF: Refactor sparse HDF5 read / write #28456

Closed

jorisvandenbossche reviewed Sep 16, 2019

View reviewed changes

added new legacy pickle files

f1afc8f

WillAyd requested changes Sep 16, 2019

View reviewed changes

WillAyd reviewed Sep 16, 2019

View reviewed changes

WillAyd reviewed Sep 17, 2019

View reviewed changes

pandas/compat/pickle_compat.py Outdated Show resolved Hide resolved

TomAugspurger added 9 commits September 17, 2019 08:39

Merge remote-tracking branch 'upstream/master' into remove-sparse

7742e36

shim

008931a

shim

04bf466

revert io changes

a4a21ae

warning for sparse

a8b0d65

Fixup typing

77b7da3

format

d265ba9

fixup typing

c2a9514

0.24.0 todo

0c02b2a

jorisvandenbossche approved these changes Sep 17, 2019

View reviewed changes

TomAugspurger merged commit 0a082d4 into pandas-dev:master Sep 18, 2019

TomAugspurger deleted the remove-sparse branch September 18, 2019 14:02

jreback mentioned this pull request Nov 22, 2019

DEPR: deprecations log for removed issues #13777

Closed

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

DEPR: Remove SparseSeries and SparseDataFrame (pandas-dev#28425)

04c4abc

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

DEPR: Remove SparseSeries and SparseDataFrame (pandas-dev#28425)

e045526

Remove SparseSeries and SparseDataFrame #28425

Remove SparseSeries and SparseDataFrame #28425

Conversation

TomAugspurger commented Sep 13, 2019 • edited Loading

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAugspurger commented Sep 13, 2019

TomAugspurger commented Sep 13, 2019

jorisvandenbossche commented Sep 13, 2019

TomAugspurger commented Sep 13, 2019

jorisvandenbossche commented Sep 13, 2019

TomAugspurger commented Sep 13, 2019

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Sep 16, 2019

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Sep 17, 2019

TomAugspurger commented Sep 17, 2019

jorisvandenbossche left a comment

Choose a reason for hiding this comment

TomAugspurger commented Sep 17, 2019 • edited Loading

jorisvandenbossche commented Sep 18, 2019

jreback commented Sep 18, 2019

jreback commented Sep 18, 2019

TomAugspurger commented Sep 18, 2019

jorisvandenbossche commented Sep 18, 2019

TomAugspurger commented Sep 13, 2019 •

edited

Loading

TomAugspurger commented Sep 17, 2019 •

edited

Loading