Skip to content
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

Drop multi-indexes when assigning to a multi-indexed variable #6798

Merged
merged 9 commits into from
Jul 21, 2022
Merged

Drop multi-indexes when assigning to a multi-indexed variable #6798

merged 9 commits into from
Jul 21, 2022

Conversation

dcherian
Copy link
Contributor

dcherian
dcherian commented Jul 16, 2022
View reviewed changes
xarray/core/coordinates.py Outdated Show resolved Hide resolved
dcherian and others added 2 commits July 16, 2022 15:19
@dcherian
fix.
3a586b7
@dcherian @andersy005
Update xarray/core/coordinates.py
44c94fa 
Co-authored-by: Anderson Banihirwe <axbanihirwe@ualr.edu>
dcherian
dcherian commented Jul 16, 2022
View reviewed changes
xarray/core/coordinates.py Outdated
)
for k in idx_coord_names:
if isinstance(self._data, Dataset):
del self._data._variables[k]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a helper function to drop "associated variables" on DataArrays and Datasets?

Copy link
Member

Choose a reason for hiding this comment

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

Like for _update_coords(), you could implement DatasetCoordinates._drop_coords() and DataArrayCoordinates._drop_coords() and call self._drop_coords() from here?

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 like it!

dcherian
dcherian commented Jul 19, 2022
View reviewed changes
xarray/core/coordinates.py
f"This will raise an error in the future. Use `.drop_vars({idx_coord_names!r})` before "
"assigning new coordinate values.",
DeprecationWarning,
stacklevel=4,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be 4 for assign_coords and 5 for Dataset.assign but perhaps this is OK.

@dcherian dcherian requested a review from benbovy July 19, 2022 15:28
benbovy
benbovy reviewed Jul 20, 2022
View reviewed changes
Copy link
Member

@benbovy benbovy left a comment

Choose a reason for hiding this comment

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

This is looking good @dcherian, thanks for fixing this! A few comments below.

xarray/core/coordinates.py Outdated Show resolved Hide resolved
xarray/core/coordinates.py Outdated Show resolved Hide resolved
xarray/core/dataset.py Outdated Show resolved Hide resolved
@dcherian dcherian requested a review from benbovy July 20, 2022 14:58
@dcherian dcherian mentioned this pull request Jul 20, 2022
Closed
benbovy
benbovy approved these changes Jul 21, 2022
View reviewed changes
Copy link
Member

@benbovy benbovy left a comment

Choose a reason for hiding this comment

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

LGTM!

@dcherian
Copy link
Contributor Author

Thanks @benbovy !

@dcherian dcherian merged commit 4a52799 into pydata:main Jul 21, 2022
@dcherian dcherian deleted the fix-multiindex-propagate branch July 21, 2022 14:46
dcherian added a commit to keewis/xarray that referenced this pull request Jul 22, 2022
@dcherian
Merge branch 'main' into duckarray-tests
21a5838 
* main: (313 commits)
  Update whats-new
  Release notes for v2022.06.0 (pydata#6815)
  Drop multi-indexes when assigning to a multi-indexed variable (pydata#6798)
  Support NumPy array API (experimental) (pydata#6804)
  Add cumsum to DatasetGroupBy (pydata#6525)
  Refactor groupby binary ops code. (pydata#6789)
  Update DataArray.rename + docu (pydata#6665)
  Switch to T_DataArray and T_Dataset in concat (pydata#6784)
  Fix typos found by codespell (pydata#6794)
  Update groupby attrs tests (pydata#6787)
  Update map_blocks to use chunksizes property. (pydata#6776)
  Fix `DataArrayRolling.__iter__` with `center=True` (pydata#6744)
  [test-upstream] Update flox repo URL (pydata#6780)
  Move _infer_meta_data and _parse_size to utils (pydata#6779)
  Make the `sel` error more descriptive when `method` is unset (pydata#6774)
  Move Rolling tests to their own testing module (pydata#6777)
  [pre-commit.ci] pre-commit autoupdate (pydata#6773)
  move da and ds fixtures to conftest.py (pydata#6730)
  Bump EnricoMi/publish-unit-test-result-action from 1 to 2 (pydata#6770)
  Type shape methods (pydata#6767)
  ...
@benbovy benbovy mentioned this pull request Aug 31, 2022
Merged
5 tasks
benbovy added a commit to benbovy/xarray that referenced this pull request Sep 28, 2022
@benbovy
DeprecationWarning -> FutureWarning
ee9b027 
The `DeprecationWarning` is ignored by default for `.assign_coords` due
to the
stacklevel (pydata#6798 (comment))

Use `FutureWarning` instead to show the warning for both `.assign` and
`.assign_coords`.
@benbovy benbovy mentioned this pull request Sep 28, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andersy005 andersy005 andersy005 left review comments

@benbovy benbovy benbovy 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.

Dropping a MultiIndex variable raises an error after explicit indexes refactor
3 participants
@dcherian @benbovy @andersy005