Skip to content

BUG: fix SparseSeries reindex by using Series implementation #15461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/source/whatsnew/v0.20.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,10 @@ Bug Fixes
- Bug in ``Rolling.quantile`` function that caused a segmentation fault when called with a quantile value outside of the range [0, 1] (:issue:`15463`)


- Bug in ``SparseSeries.reindex`` on single level with list of length 1 (:issue:`15447`)



- Bug in the display of ``.info()`` where a qualifier (+) would always be displayed with a ``MultiIndex`` that contains only non-strings (:issue:`15245`)
- Bug in ``pd.read_msgpack()`` in which ``Series`` categoricals were being improperly processed (:issue:`14901`)
- Bug in ``Series.ffill()`` with mixed dtypes containing tz-aware datetimes. (:issue:`14956`)
Expand Down
24 changes: 5 additions & 19 deletions pandas/sparse/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
_coo_to_sparse_series)


_shared_doc_kwargs = dict(klass='SparseSeries',
_shared_doc_kwargs = dict(axes='index', klass='SparseSeries',
axes_single_arg="{0, 'index'}")

# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -570,27 +570,13 @@ def copy(self, deep=True):
return self._constructor(new_data, sparse_index=self.sp_index,
fill_value=self.fill_value).__finalize__(self)

@Appender(generic._shared_docs['reindex'] % _shared_doc_kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave the signature as it was
use a shared_doc to construct the doc-strings (see how its done in Series itself).

def reindex(self, index=None, method=None, copy=True, limit=None,
**kwargs):
"""
Conform SparseSeries to new Index

See Series.reindex docstring for general behavior

Returns
-------
reindexed : SparseSeries
"""
new_index = _ensure_index(index)

if self.index.equals(new_index):
if copy:
return self.copy()
else:
return self
return self._constructor(self._data.reindex(new_index, method=method,
limit=limit, copy=copy),
index=new_index).__finalize__(self)
return super(SparseSeries, self).reindex(index=index, method=method,
copy=copy, limit=limit,
**kwargs)

def sparse_reindex(self, new_index):
"""
Expand Down
34 changes: 34 additions & 0 deletions pandas/tests/sparse/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,11 @@ def test_loc(self):
exp = orig.loc[[1, 3, 4, 5]].to_sparse()
tm.assert_sp_series_equal(result, exp)

# single element list (GH 15447)
result = sparse.loc[['A']]
exp = orig.loc[['A']].to_sparse()
tm.assert_sp_series_equal(result, exp)

# dense array
result = sparse.loc[orig % 2 == 1]
exp = orig.loc[orig % 2 == 1].to_sparse()
Expand Down Expand Up @@ -537,6 +542,35 @@ def test_loc_slice(self):
orig.loc['A':'B'].to_sparse())
tm.assert_sp_series_equal(sparse.loc[:'B'], orig.loc[:'B'].to_sparse())

def test_reindex(self):
# GH 15447
orig = self.orig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the issue number

sparse = self.sparse

res = sparse.reindex([('A', 0), ('C', 1)])
exp = orig.reindex([('A', 0), ('C', 1)]).to_sparse()
tm.assert_sp_series_equal(res, exp)

# On specific level:
res = sparse.reindex(['A', 'C', 'B'], level=0)
exp = orig.reindex(['A', 'C', 'B'], level=0).to_sparse()
tm.assert_sp_series_equal(res, exp)

# single element list (GH 15447)
res = sparse.reindex(['A'], level=0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you test copy and other params (limit)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test for copy, others should be already tested in pandas/test/series/missing.py, e.g. line 659.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above, this is True for Series, but NOT for SparseSeries

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant line 640

exp = orig.reindex(['A'], level=0).to_sparse()
tm.assert_sp_series_equal(res, exp)

with tm.assertRaises(TypeError):
# Incomplete keys are not accepted for reindexing:
sparse.reindex(['A', 'C'])

# "copy" argument:
res = sparse.reindex(sparse.index, copy=True)
exp = orig.reindex(orig.index, copy=True).to_sparse()
tm.assert_sp_series_equal(res, exp)
self.assertIsNot(sparse, res)


class TestSparseDataFrameIndexing(tm.TestCase):

Expand Down