API: Return sparse objects always for cumsum #14771

gfyoung · 2016-11-30T10:47:11Z

Always return SparseArray and SparseSeries for SparseArray.cumsum() and SparseSeries.cumsum() respectively, regardless of fill_value.

Close #12855.

sinhrks · 2016-11-30T12:24:29Z

pandas/sparse/array.py

        """
        nv.validate_cumsum(args, kwargs)

-        # TODO: gh-12855 - return a SparseArray here
        if notnull(self.fill_value):


can use self._null_fill_value.

sinhrks · 2016-11-30T12:25:54Z

pandas/sparse/series.py


        Returns
        -------
-        cumsum : SparseSeries if `self` has a null `fill_value` and a
-                 generic Series otherwise
+        cumsum : SparseSeries
        """
        nv.validate_cumsum(args, kwargs)
        new_array = SparseArray.cumsum(self.values)


Because self.values is SparseArray, this wrapping looks unnecessary.

Makes sense. Done.

codecov-io · 2016-12-01T23:06:04Z

Current coverage is 85.26% (diff: 100%)

Merging #14771 into master will increase coverage by <.01%

@@             master     #14771   diff @@
==========================================
  Files           144        144          
  Lines         50981      50983     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43470      43472     +2   
  Misses         7511       7511          
  Partials          0          0

Powered by Codecov. Last update 81a2f79...83314fc

jreback · 2016-12-04T17:31:25Z

pandas/sparse/array.py

+
+        Parameters
+        ----------
+        axis : int


is this for compat? why are you writing it in the doc-string? (rather than raising on it if its not 0)

@jreback : The parameter was in the signature but undocumented. I believe it is for compat since SparseDataFrame actually uses this argument.

@jreback : Should we just do validation as part of this PR for both SparseArray and SparseSeries then?

jreback · 2016-12-04T17:32:30Z

pandas/sparse/series.py


        Returns
        -------
-        cumsum : SparseSeries if `self` has a null `fill_value` and a
-                 generic Series otherwise
+        cumsum : SparseSeries
        """
        nv.validate_cumsum(args, kwargs)


e.g. I would do axis = self._get_axis_number(axis) (which validates).

and something similar to SparseArray.

We normally don't list these in the doc-string Parameters (even though they are in the signature)

Fair enough, though it was misleading because I had initially assumed the axis parameter did something, even though it doesn't do anything in this implementation.

jreback · 2016-12-04T17:33:01Z

pandas/sparse/array.py

-        if notnull(self.fill_value):
-            return self.to_dense().cumsum()
+        if not self._null_fill_value:
+            return SparseArray(self.to_dense()).cumsum()


side thing. we should use _constructor (and define it for SparseArray).

can make a separate issue / do later

Sounds good as a follow-up.

jreback · 2016-12-05T23:15:28Z

pandas/sparse/series.py

+        ----------
+        axis : int
+            Axis over which to perform the cumulative summation. This
+            parameter is ignored because `SparseSeries` is 1-D, but it


is there a reason you are not matching the axis parameter for Series?

Help on function cumsum in module pandas.core.series: cumsum(self, axis=None, skipna=True, *args, **kwargs) Return cumulative sum over requested axis. Parameters ---------- axis : {index (0)} skipna : boolean, default True Exclude NA/null values. If an entire row/column is NA, the result will be NA Returns ------- cumsum : scalar

No reason really. Done.

gfyoung · 2016-12-08T18:49:33Z

@jreback : I addressed all of the comments, and everything is passing. Ready to merge if there are no other concerns.

jreback · 2016-12-10T15:43:34Z

pandas/sparse/series.py

+
+        Parameters
+        ----------
+        axis : 0 or None


it cannot be None, must be 0 (as you set the default)

jreback · 2016-12-10T15:43:55Z

pandas/sparse/series.py

-        # TODO: gh-12855 - return a SparseSeries here
-        return Series(new_array, index=self.index).__finalize__(self)
+        if axis is not None:
+            self._get_axis_number(axis)  # Unused but hould be valid!


we only do

axis=self._get_axis_number(axis) and remove the comment.

Always return SparseArray and SparseSeries for SparseArray.cumsum() and SparseSeries.cumsum() respectively, regardless of fill_value. Close pandas-devgh-12855.

gfyoung · 2016-12-14T23:00:36Z

@jreback : Applied requested changes, and all is green. Ready to merge if there are no other concerns.

jreback · 2016-12-15T00:30:36Z

lgtm.

jreback · 2016-12-15T11:28:37Z

thanks!

Always return `SparseArray` and `SparseSeries` for `SparseArray.cumsum()` and `SparseSeries.cumsum()` respectively, regardless of `fill_value`. Closes pandas-dev#12855. Author: gfyoung <[email protected]> Closes pandas-dev#14771 from gfyoung/sparse-return-type and squashes the following commits: 83314fc [gfyoung] API: Return sparse objects always for cumsum

jreback added Numeric Operations Arithmetic, Comparison, and Logical operations Sparse Sparse Data Type labels Nov 30, 2016

sinhrks suggested changes Nov 30, 2016

View reviewed changes

gfyoung force-pushed the sparse-return-type branch from 22049b4 to 59fd828 Compare November 30, 2016 17:15

sinhrks approved these changes Nov 30, 2016

View reviewed changes

gfyoung force-pushed the sparse-return-type branch from 59fd828 to b2af5ee Compare December 1, 2016 02:49

chrisaycock mentioned this pull request Dec 1, 2016

ENH: merge_asof() has type specializations and can take multiple 'by' parameters (#13936) #14783

Closed

4 tasks

gfyoung force-pushed the sparse-return-type branch from b2af5ee to 6ab170f Compare December 1, 2016 23:06

gfyoung force-pushed the sparse-return-type branch from 6ab170f to 39eb909 Compare December 2, 2016 03:32

jreback reviewed Dec 4, 2016

View reviewed changes

jreback reviewed Dec 5, 2016

View reviewed changes

gfyoung force-pushed the sparse-return-type branch 2 times, most recently from ac042da to a472432 Compare December 6, 2016 16:34

jreback reviewed Dec 10, 2016

View reviewed changes

API: Return sparse objects always for cumsum

83314fc

Always return SparseArray and SparseSeries for SparseArray.cumsum() and SparseSeries.cumsum() respectively, regardless of fill_value. Close pandas-devgh-12855.

gfyoung force-pushed the sparse-return-type branch from a472432 to 83314fc Compare December 11, 2016 02:09

jreback added this to the 0.20.0 milestone Dec 15, 2016

jreback closed this in 33e11ad Dec 15, 2016

gfyoung deleted the sparse-return-type branch December 15, 2016 16:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: Return sparse objects always for cumsum #14771

API: Return sparse objects always for cumsum #14771

gfyoung commented Nov 30, 2016

sinhrks Nov 30, 2016

gfyoung Nov 30, 2016

sinhrks Nov 30, 2016

gfyoung Nov 30, 2016 •

edited

Loading

codecov-io commented Dec 1, 2016 •

edited

Loading

jreback Dec 4, 2016

gfyoung Dec 4, 2016

gfyoung Dec 4, 2016 •

edited

Loading

jreback Dec 4, 2016

gfyoung Dec 4, 2016

jreback Dec 4, 2016

jreback Dec 4, 2016

gfyoung Dec 4, 2016

jreback Dec 5, 2016 •

edited

Loading

gfyoung Dec 6, 2016

gfyoung commented Dec 8, 2016

jreback Dec 10, 2016

gfyoung Dec 11, 2016

jreback Dec 10, 2016

gfyoung Dec 11, 2016

gfyoung commented Dec 14, 2016

jreback commented Dec 15, 2016

jreback commented Dec 15, 2016

API: Return sparse objects always for cumsum #14771

API: Return sparse objects always for cumsum #14771

Conversation

gfyoung commented Nov 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Nov 30, 2016 • edited Loading

Choose a reason for hiding this comment

codecov-io commented Dec 1, 2016 • edited Loading

Current coverage is 85.26% (diff: 100%)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Dec 4, 2016 • edited Loading

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

jreback Dec 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung commented Dec 8, 2016

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

gfyoung commented Dec 14, 2016

jreback commented Dec 15, 2016

jreback commented Dec 15, 2016

gfyoung Nov 30, 2016 •

edited

Loading

codecov-io commented Dec 1, 2016 •

edited

Loading

gfyoung Dec 4, 2016 •

edited

Loading

jreback Dec 5, 2016 •

edited

Loading