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

mfdataset, concat now support the 'join' kwarg. #3102

Merged
merged 13 commits into from
Aug 7, 2019

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Jul 12, 2019

I won't work on it for the next few days if someone else wants to take over...

@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #3102 into master will increase coverage by 0.24%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3102      +/-   ##
==========================================
+ Coverage   95.75%   95.99%   +0.24%     
==========================================
  Files          63       63              
  Lines       12811    12799      -12     
==========================================
+ Hits        12267    12287      +20     
+ Misses        544      512      -32

@dcherian dcherian changed the title [WIP] mfdatset, concat now support the 'join' kwarg. [WIP] mfdataset, concat now support the 'join' kwarg. Jul 12, 2019
Copy link
Contributor Author

@dcherian dcherian 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 ready for review.

I really needed tests to make sure I added join in all the required places. Can someone check to make sure I've added enough tests please?

xarray/tests/test_backends.py Show resolved Hide resolved
@dcherian dcherian changed the title [WIP] mfdataset, concat now support the 'join' kwarg. mfdataset, concat now support the 'join' kwarg. Jul 17, 2019
@dcherian dcherian mentioned this pull request Aug 1, 2019
3 tasks
* master:
  More annotations in Dataset (pydata#3112)
  Hotfix for case of combining identical non-monotonic coords (pydata#3151)
  changed url for rasterio network test (pydata#3162)
@dcherian dcherian requested a review from TomNicholas August 2, 2019 14:47
Copy link
Member

@TomNicholas TomNicholas left a comment

Choose a reason for hiding this comment

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

Can someone check to make sure I've added enough tests please?

I think your tests cover the possible cases well, but they are all testing at the level of open_mfdataset - there aren't any which call combine_by_coords or combine_nested directly. Not sure if that's a problem.

xarray/backends/api.py Outdated Show resolved Hide resolved
xarray/tests/test_backends.py 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)
@pep8speaks
Copy link

pep8speaks commented Aug 6, 2019

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 2019-08-06 14:38:44 UTC

@dcherian
Copy link
Contributor Author

dcherian commented Aug 6, 2019

Thanks for the review @TomNicholas. I've updated the docstrings and added tests to test_combine.py. I think this should be good to go.

@TomNicholas TomNicholas merged commit 04597a8 into pydata:master Aug 7, 2019
dcherian added a commit to yohai/xarray that referenced this pull request Aug 7, 2019
* 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)
dcherian added a commit to dcherian/xarray that referenced this pull request Aug 8, 2019
* 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)
@dcherian dcherian deleted the concat-join branch August 9, 2019 16:55
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.

concat automagically outer-joins coordinates
3 participants