Skip to content

Fix conversion of multiindexed pandas objects to sparse xarray objects #4088

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 26, 2020
Merged

Fix conversion of multiindexed pandas objects to sparse xarray objects #4088

merged 6 commits into from
May 26, 2020

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented May 22, 2020 •
edited
Loading

Doesn't have a proper test. Need some help here. cc @bnaul

@pep8speaks
Copy link

pep8speaks commented May 22, 2020 •
edited
Loading

Hello @dcherian! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-05-25 22:59:09 UTC

dcherian added 5 commits May 22, 2020 07:59
@dcherian
lint
9551063
@dcherian
fix whats-new
8ee69dd
@dcherian
fix test
63061b6
@dcherian
minor whats-new
ba7b47a
@dcherian
Merge remote-tracking branch 'upstream/master' into fix-sparse
c9a4205 
* upstream/master:
  Improve interp performance (pydata#4069)
  Auto chunk (pydata#4064)
  xr.cov() and xr.corr() (pydata#4089)
  allow multiindex levels in plots (pydata#3938)
  Fix bool weights (pydata#4075)
  fix dangerous default arguments (pydata#4006)
@dcherian
Copy link
Contributor Author

Ready for review. I added a test that checks if the coords for the underlying sparse object are the same when created in two ways

  1. from_series(..., sparse=False).data converted to sparse using sparse.COO.from_numpy
  2. from_series(..., sparse=True).data

@dcherian dcherian mentioned this pull request May 25, 2020
Closed
23 tasks
fujiisoup
fujiisoup approved these changes May 25, 2020
View reviewed changes
Copy link
Member

@fujiisoup fujiisoup left a comment

Choose a reason for hiding this comment

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

Looks good to me :)
Thanks, @dcherian

shoyer
shoyer reviewed May 26, 2020
View reviewed changes
xarray/core/dataset.py
@@ -4537,7 +4537,7 @@ def _set_sparse_data_from_dataframe(
idx = dataframe.index
if isinstance(idx, pd.MultiIndex):
coords = np.stack([np.asarray(code) for code in idx.codes], axis=0)
is_sorted = idx.is_lexsorted
is_sorted = idx.is_lexsorted()
Copy link
Member

Choose a reason for hiding this comment

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

Good catch! This is a little embarrassing :)

Copy link
Contributor Author

@dcherian dcherian May 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

:) I blame Pandas API. Index.is_sorted and Index.is_monotonic are both properties. So expecting is_lexsorted to be a property is totally reasonable!

@dcherian
Copy link
Contributor Author

Thanks for the reviews

@dcherian dcherian merged commit e5cc19c into pydata:master May 26, 2020
@dcherian dcherian deleted the fix-sparse branch May 26, 2020 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shoyer shoyer shoyer approved these changes

@fujiisoup fujiisoup fujiisoup approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Sparse DataArray indexing gives incorrect results
4 participants
@dcherian @pep8speaks @shoyer @fujiisoup