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

preserve original dimension, coordinate and variable order in concat #4419

Merged
merged 9 commits into from
Sep 19, 2020
Merged

preserve original dimension, coordinate and variable order in concat #4419

merged 9 commits into from
Sep 19, 2020

Conversation

kmuehlbauer
Copy link
Contributor

@kmuehlbauer kmuehlbauer commented Sep 14, 2020 •
edited by keewis
Loading

The problem is that the result_vars-dict is filled in two separate loops, handling variables and coordinates (AFAICT). This can change the ordering of dimensions and coordinates depending over which dimensions is concatenated.

This fix hooks into the second loop and reinsert the variable in question at the appropriate position by popping it out of the result_vars. There might be a more elegant solution to this.

@kmuehlbauer kmuehlbauer mentioned this pull request Sep 14, 2020
Merged
4 tasks
keewis
keewis approved these changes Sep 14, 2020
View reviewed changes
Copy link
Collaborator

@keewis keewis left a comment

Choose a reason for hiding this comment

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

except from a few stylistic issues (which I think make reading the code easer), this looks good to me.

xarray/tests/test_concat.py Outdated Show resolved Hide resolved
keewis
keewis reviewed Sep 14, 2020
View reviewed changes
xarray/tests/test_concat.py Outdated Show resolved Hide resolved
@kmuehlbauer
Copy link
Contributor Author

@keewis @dcherian I've tried to get rid of the ValueError: dimension 'time' already exists as a scalar variable in the test by using 1D numpy array. Hope this fits better now style-wise.

Also added whats-new.rst entry. This is now good to go from my side. Thanks for taking care.

@keewis
Copy link
Collaborator

keewis commented Sep 15, 2020 •
edited
Loading

thanks and sorry for that, I guess the error was caused by me proposing changes without testing they would actually work

@keewis keewis mentioned this pull request Sep 18, 2020
Closed
@max-sixty max-sixty merged commit 7d389db into pydata:master Sep 19, 2020
@max-sixty
Copy link
Collaborator

Thanks @kmuehlbauer !

@keewis keewis mentioned this pull request Sep 19, 2020
Merged
1 task
dcherian added a commit to dcherian/xarray that referenced this pull request Oct 9, 2020
@dcherian
Merge branch 'propagate-attrs' of github.com:dcherian/xarray into pro…
1f998f0 
…pagate-attrs

* 'propagate-attrs' of github.com:dcherian/xarray: (22 commits)
  silence sphinx warnings about broken rst (pydata#4448)
  Xarray open_mfdataset with engine Zarr (pydata#4187)
  Fix release notes formatting (pydata#4443)
  fix typo in io.rst (pydata#4250)
  Fix typo (pydata#4181)
  Fix release notes typo
  New whatsnew section
  Add notes re doctests (pydata#4440)
  Fixed dask.optimize on datasets (pydata#4438)
  Release notes for 0.16.1 (pydata#4435)
  Small updates to How-to-release + lint (pydata#4436)
  Fix doctests (pydata#4439)
  add a ci for doctests (pydata#4437)
  preserve original dimension, coordinate and variable order in ``concat`` (pydata#4419)
  Fix for h5py deepcopy issues (pydata#4426)
  Keep the original ordering of the coordinates (pydata#4409)
  Clearer Vectorized Indexing example (pydata#4433)
  Revert "Fix optimize for chunked DataArray (pydata#4432)" (pydata#4434)
  Fix optimize for chunked DataArray (pydata#4432)
  fix doc dataarray to netcdf (pydata#4424)
  ...
@kmuehlbauer kmuehlbauer deleted the preserve-var-order-concat branch October 9, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keewis keewis keewis approved these changes

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

[BUG] xr.concat inverts coordinates order concat changes variable order
4 participants
@kmuehlbauer @keewis @max-sixty @dcherian