Skip to content

Fix indexing, reindex on all-sparse SparseArray. #35287

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

Merged
merged 5 commits into from
Jul 16, 2020

Conversation

TomAugspurger
Copy link
Contributor

@TomAugspurger TomAugspurger commented Jul 15, 2020

Closes #35286.

Also added a regression tests for the issue reported there.

@TomAugspurger TomAugspurger added this to the 1.1 milestone Jul 15, 2020
@TomAugspurger TomAugspurger added the Regression Functionality that used to work in a prior pandas version label Jul 15, 2020
@TomAugspurger
Copy link
Contributor Author

Looks like #34908 built on the buggy functionality. I may need to revert it as well.

Comment on lines -44 to -47
@pytest.fixture(params=[0, np.nan])
def data_zeros(request):
return SparseArray(np.zeros(100, dtype=int), fill_value=request.param)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this was doing anything on master.

Copy link
Contributor Author

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

a089991 hopefully has the proper fix.

@@ -399,31 +399,3 @@ def test_item(self, data):

with pytest.raises(ValueError, match=msg):
s.item()

def test_boolean_mask_frame_fill_value(self, data):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that these tests were passing regardless of the change, so I'm not sure they were testing what we expected. I've left them removed.

@TomAugspurger TomAugspurger changed the title Revert 34158 Fix indexing, reindex on all-sparse SparseArray. Jul 15, 2020
@jreback jreback added the Sparse Sparse Data Type label Jul 15, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. wasn't clear if you had additional changes. merge when ready.

@TomAugspurger
Copy link
Contributor Author

Thanks, I think we're good here.

@TomAugspurger TomAugspurger merged commit 92c9e69 into pandas-dev:master Jul 16, 2020
@TomAugspurger TomAugspurger deleted the revert-34158 branch July 16, 2020 11:17
fangchenli pushed a commit to fangchenli/pandas that referenced this pull request Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Functionality that used to work in a prior pandas version Sparse Sparse Data Type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REGR: reindex with sparse data
2 participants