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.

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

Add join='override' #3175

Merged
merged 19 commits into from
Aug 16, 2019
Merged

Add join='override' #3175

merged 19 commits into from
Aug 16, 2019

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Aug 1, 2019

This adds join='override' which checks that indexes along a dimension are of the same size and overwrites those indices with indices from the first object.

Definitely need help, feedback.

xarray/core/dataarray.py Outdated Show resolved Hide resolved
xarray/core/alignment.py Outdated Show resolved Hide resolved
* master:
  enable sphinx.ext.napoleon (pydata#3180)
  remove type annotations from autodoc method signatures (pydata#3179)
  Fix regression: IndexVariable.copy(deep=True) casts dtype=U to object (pydata#3095)
  Fix distributed.Client.compute applied to DataArray (pydata#3173)
* master:
  pyupgrade one-off run (pydata#3190)
  mfdataset, concat now support the 'join' kwarg. (pydata#3102)
  reduce the size of example dataset in dask docs (pydata#3187)
  add climpred to related-projects (pydata#3188)
  bump rasterio to 1.0.24 in doc building environment (pydata#3186)
  More annotations (pydata#3177)
  Support for __array_function__ implementers (sparse arrays) [WIP] (pydata#3117)
  Internal clean-up of isnull() to avoid relying on pandas (pydata#3132)
  Call darray.compute() in plot() (pydata#3183)
  BUG: fix + test open_mfdataset fails on variable attributes with list… (pydata#3181)
* commit 'f172c673':
  ENH: Scatter plots of one variable vs another (pydata#2277)
  Escape code markup (pydata#3189)
@crusaderky
Copy link
Contributor

crusaderky commented Aug 9, 2019

Hi,

This doesn't close #2039 because

  • open_mfdataset does not offer a 'join' parameter to be propagated to align [EDIT] I just saw mfdataset, concat now support the 'join' kwarg. #3102, sorry
  • even if it did, by when open_mfdataset invokes align time has already been wasted because indices, as of today, are eagerly loaded by open_dataset

@dcherian
Copy link
Contributor Author

dcherian commented Aug 9, 2019

Ah, good point. Do you have any thoughts on the implementation? or on the compat='override' proposal in #2064 (comment)

@crusaderky
Copy link
Contributor

crusaderky commented Aug 9, 2019

Lazy-loading indices, plus this and #3102, would probably solve #2039.
Without lazy-loading, an ad-hoc hack needs to be implemented in open_mfdataset directly...

* upstream/master:
  Ignore example.grib.0112.idx file (pydata#3198)
  small updates to the contributing.rst (it could use more) (pydata#3193)
  Remove future statements (pydata#3194)
  update instructions (pydata#3195)
@dcherian dcherian changed the title [WIP] Add join='override' Add join='override' Aug 12, 2019
@dcherian
Copy link
Contributor Author

Ready for review.

Copy link
Member

@shoyer shoyer left a comment

Choose a reason for hiding this comment

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

I think this general approach makes sense 👍

xarray/core/alignment.py Outdated Show resolved Hide resolved
xarray/core/alignment.py Outdated Show resolved Hide resolved
xarray/core/alignment.py Outdated Show resolved Hide resolved
Copy link
Member

@shoyer shoyer left a comment

Choose a reason for hiding this comment

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

Very clean, I like it!

xarray/core/alignment.py Outdated Show resolved Hide resolved
xarray/core/alignment.py Outdated Show resolved Hide resolved
@shoyer
Copy link
Member

shoyer commented Aug 15, 2019

LGTM -- anyone else have opinions here?

@max-sixty
Copy link
Collaborator

Yes agree! Thanks @dcherian !

@dcherian
Copy link
Contributor Author

Great, thanks!

@dcherian dcherian merged commit 7866587 into pydata:master Aug 16, 2019
@dcherian dcherian deleted the join-override branch August 16, 2019 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants