Skip to content

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?

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

* 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