CLN: Removed SparsePanel #13778

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Member

gfyoung commented Jul 24, 2016

Title is self-explanatory. Picks up where #11157 left off.

@gfyoung gfyoung commented on an outdated diff Jul 24, 2016

doc/source/whatsnew/v0.19.0.txt
@@ -330,6 +330,7 @@ API changes
~~~~~~~~~~~
+- ``Panel.to_sparse`` will raise a ``NotImplementedError`` exception when called (:issue:``)
@gfyoung

gfyoung Jul 24, 2016

Member

Note to self: add PR number once Travis passes

@gfyoung gfyoung commented on an outdated diff Jul 24, 2016

doc/source/whatsnew/v0.19.0.txt
@@ -615,6 +616,7 @@ Deprecations
Removal of prior version deprecations/changes
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- The ``SparsePanel`` class has been removed (:issue:``)
@gfyoung

gfyoung Jul 24, 2016

Member

Note to self: add PR number once Travis passes

Contributor

jreback commented Jul 24, 2016

this is in slight conflict with #13776 as I needed to add a line to catch an errant warning about SparsePanel (which you should then remove). But can sort that later I guess.

@gfyoung gfyoung commented on the diff Jul 24, 2016

bench/bench_sparse.py
-dwp = Panel.fromDict({'foo': dm})
-# sdf = SparseDataFrame(data)
-
-
-lp = stack_sparse_frame(sdf)
-
-
-swp = SparsePanel({'A': sdf})
-swp = SparsePanel({'A': sdf,
- 'B': sdf,
- 'C': sdf,
- 'D': sdf})
-
-y = sdf
-x = SparsePanel({'x1': sdf + new_data_like(sdf) / 10,
- 'x2': sdf + new_data_like(sdf) / 10})
@gfyoung

gfyoung Jul 24, 2016

Member

The benchmark centres around this SparsePanel, which if removed, renders this benchmark essentially moot.

@jorisvandenbossche

jorisvandenbossche Jul 24, 2016

Owner

that's fine (we are going to remove those benchs shortly anyway, after checking the content, there is an issue for it. And this one doesn't contain any content that should be moved, so fine!)

jreback added the Deprecate label Jul 24, 2016

@jreback jreback commented on the diff Jul 24, 2016

doc/source/sparse.rst
@@ -77,9 +77,8 @@ distinct from the ``fill_value``:
sparr = pd.SparseArray(arr)
sparr
-Like the indexed objects (SparseSeries, SparseDataFrame, SparsePanel), a
-``SparseArray`` can be converted back to a regular ndarray by calling
-``to_dense``:
+Like the indexed objects (SparseSeries, SparseDataFrame), a ``SparseArray``
+can be converted back to a regular ndarray by calling ``to_dense``:
@jreback

jreback Jul 24, 2016 edited

Contributor

maybe add a warning note somewhere that SparsePanel was deprecated in 0.19.0

@gfyoung

gfyoung Jul 24, 2016 edited

Member

You mean "removed in 0.19.0" ?

@jreback

jreback Jul 25, 2016

Contributor

yes

@jreback jreback commented on the diff Jul 24, 2016

pandas/io/tests/test_packers.py
@@ -543,25 +543,6 @@ def test_sparse_frame(self):
self._check_roundtrip(ss3, tm.assert_frame_equal,
check_frame_type=True)
- def test_sparse_panel(self):
-
@jreback

jreback Jul 24, 2016

Contributor

hmm I think we have legacy pickled / msgpacks that DO serialize this, so need to ignore that NotImplementedError when checking (IOW in io/tests/test_pickle.py)

@gfyoung

gfyoung Jul 24, 2016 edited

Member

I don't really follow. How are we supposed to test pickling sparse_panel if it doesn't exist? Are we supposed to just check the Panel object itself then?

@jreback

jreback Jul 24, 2016 edited

Contributor

well if u have a class that doesn't exist and you try to unpickle it will fail

now I don't know if these are actually saved in pickles (maybe not) so may be moot

@gfyoung

gfyoung Jul 24, 2016

Member

As I mentioned below, I don't believe that there are any pickles for SparsePanel.

@jreback jreback commented on the diff Jul 24, 2016

pandas/io/tests/test_pytables.py
@@ -2665,23 +2665,6 @@ def test_sparse_frame(self):
self._check_double_roundtrip(ss3, tm.assert_frame_equal,
check_frame_type=True)
- def test_sparse_panel(self):
@jreback

jreback Jul 24, 2016

Contributor

same here, these exist in the legacy file (I think)

@gfyoung

gfyoung Jul 24, 2016

Member

GitHub search says otherwise. Also, not in the generate_legacy_storage_files.py file.

@jreback

jreback Jul 24, 2016

Contributor

it's possible these were never put there

@gfyoung

gfyoung Jul 24, 2016 edited

Member

Well, unless specific files can be pointed out, a GitHub search is sufficient to convince me that they are not there. Not to mention, there would have been test failures because all legacy pickles should be tested (this was the same issue with the Categorical levels thing).

@jreback

jreback Jul 24, 2016

Contributor

no a search is not sufficient
but if we don't have tests which fail when u remove it - it is fine

you always have to assume other people don't do things and test everything

Member

gfyoung commented Jul 24, 2016

@jreback : what error catching are you referring to in your PR?

codecov-io commented Jul 25, 2016 edited

Current coverage is 85.23% (diff: 75.00%)

Merging #13778 into master will decrease coverage by 0.10%

@@             master     #13778   diff @@
==========================================
  Files           141        140     -1   
  Lines         50711      50415   -296   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          43275      42969   -306   
- Misses         7436       7446    +10   
  Partials          0          0          

Powered by Codecov. Last update 3fdcea6...f3fa93b

@jreback jreback and 1 other commented on an outdated diff Jul 25, 2016

doc/source/sparse.rst
@@ -15,13 +15,13 @@
Sparse data structures
**********************
-We have implemented "sparse" versions of Series, DataFrame, and Panel. These
-are not sparse in the typical "mostly 0". You can view these objects as being
-"compressed" where any data matching a specific value (NaN/missing by default,
-though any value can be chosen) is omitted. A special ``SparseIndex`` object
-tracks where data has been "sparsified". This will make much more sense in an
-example. All of the standard pandas data structures have a ``to_sparse``
-method:
+We have implemented "sparse" versions of Series and DataFrame (there used to be
+one for Panel but was removed in 0.19.0). These are not sparse in the typical
@jreback

jreback Jul 25, 2016

Contributor

put it in a separate note about the removal of the SparsePanel.

@gfyoung

gfyoung Jul 25, 2016

Member

I have a parenthetical above your comment about the removal. How about a separate paragraph underneath this one instead?

@jreback

jreback Jul 25, 2016

Contributor

put it in a warning or note, just trying to draw attention. It should be the first item in that section.

@gfyoung

gfyoung Jul 26, 2016

Member

Sounds good. Done.

@jreback jreback and 1 other commented on an outdated diff Jul 25, 2016

pandas/tests/test_panel.py
@@ -2442,7 +2403,13 @@ def test_to_string(self):
buf = StringIO()
self.panel.to_string(buf)
- @ignore_sparse_panel_future_warning
+ def test_to_sparse(self):
+ # LongPanel's panel is a DataFrame
@jreback

jreback Jul 25, 2016

Contributor

remove this comment

jreback added this to the 0.19.0 milestone Jul 25, 2016

jreback referenced this pull request Jul 25, 2016

Open

DEPR: deprecations log for removed issues #13777

65 of 65 tasks complete
Contributor

jreback commented Jul 25, 2016

lgtm @gfyoung minor doc changes. ping when green.

Contributor

jreback commented Jul 25, 2016

@gfyoung I think I took out the reference in my other PR (to SparsePanel); I'll rebase after this.

@gfyoung gfyoung CLN: Removed SparsePanel
f3fa93b

jreback closed this in 690d52c Jul 26, 2016

Contributor

jreback commented Jul 26, 2016

thanks @gfyoung

sinhrks referenced this pull request Jul 26, 2016

Closed

BUG: Slicing subclasses of SparseDataFrames. #13787

2 of 2 tasks complete

gfyoung deleted the gfyoung:remove-sparse-panel branch Jul 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment