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

Allow invalid_netcdf=True in to_netcdf() #3221

Merged
merged 11 commits into from
Aug 22, 2019

Conversation

ulijh
Copy link
Contributor

@ulijh ulijh commented Aug 15, 2019

Hi all,

I prepared a little PR which could close #2243 and would allow for a IMO clean way of writing data with complex dtypes (and others).

What do you think?

TODOs:

@ulijh ulijh changed the title Allow invalid=netcdf=True in to_netcdf() Allow invalid_netcdf=True in to_netcdf() Aug 15, 2019
Copy link
Contributor

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

Just a minor comment. LGTM but I'm not familiar with the backend bits...

This will also need a note in whats-new.rst and a note in io.rst, perhaps under "Writing encoded data"

xarray/backends/api.py Outdated Show resolved Hide resolved
xarray/backends/h5netcdf_.py Outdated Show resolved Hide resolved
xarray/core/dataset.py Outdated Show resolved Hide resolved
xarray/tests/test_backends.py Outdated Show resolved Hide resolved
@ulijh
Copy link
Contributor Author

ulijh commented Aug 16, 2019

This will also need a note in whats-new.rst and a note in io.rst, perhaps under "Writing encoded data"

Sure, I'll add that when we are happy with the code and tests!

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.

Code and tests look great, thank you!

xarray/tests/test_backends.py Outdated Show resolved Hide resolved
@ulijh
Copy link
Contributor Author

ulijh commented Aug 17, 2019

Hmm, seems like I broke the docs somehow... I someone could advice me how to fix? Thanks!

@ulijh
Copy link
Contributor Author

ulijh commented Aug 22, 2019

Hey guys, I'd like to move this forward, but the doc build failed with the message below.

  • Do I need to add something specific to make the docs build?
  • Do I need to do anything else to get this PR merged?

Thanks!

>>>-------------------------------------------------------------------------
Exception in /home/vsts/work/1/s/doc/io.rst at block ending on line 388
Specify :okexcept: as an option in the ipython:: block to suppress this message
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-17-9100cd49113c> in <module>
----> 1 da.to_netcdf("complex.nc", engine="h5netcdf", invalid_netcdf=True)

~/work/1/s/xarray/core/dataarray.py in to_netcdf(self, *args, **kwargs)
   2214             dataset = self.to_dataset()
   2215 
-> 2216         return dataset.to_netcdf(*args, **kwargs)
   2217 
   2218     def to_dict(self, data: bool = True) -> dict:

~/work/1/s/xarray/core/dataset.py in to_netcdf(self, path, mode, format, group, engine, encoding, unlimited_dims, compute, invalid_netcdf)
   1519             unlimited_dims=unlimited_dims,
   1520             compute=compute,
-> 1521             invalid_netcdf=invalid_netcdf,
   1522         )
   1523 

~/work/1/s/xarray/backends/api.py in to_netcdf(dataset, path_or_file, mode, format, group, engine, encoding, unlimited_dims, compute, multifile, invalid_netcdf)
   1052                 "unrecognized option 'invalid_netcdf' for engine %s" % engine
   1053             )
-> 1054     store = store_open(target, mode, format, group, **kwargs)
   1055 
   1056     if unlimited_dims is None:

~/work/1/s/xarray/backends/h5netcdf_.py in __init__(self, filename, mode, format, group, lock, autoclose, invalid_netcdf)
     81         invalid_netcdf=None,
     82     ):
---> 83         import h5netcdf
     84 
     85         if format not in [None, "NETCDF4"]:

ModuleNotFoundError: No module named 'h5netcdf'
<<<-------------------------------------------------------------------------

/home/vsts/work/1/s/xarray/core/dataarray.py:docstring of xarray.DataArray.integrate:12: WARNING: Unexpected indentation.
/home/vsts/work/1/s/xarray/core/dataarray.py:docstring of xarray.DataArray.interp:20: WARNING: Inline strong start-string without end-string.
/home/vsts/work/1/s/xarray/core/dataarray.py:docstring of xarray.DataArray.interpolate_na:8: WARNING: Definition list ends without a blank line; unexpected unindent.

Sphinx parallel build error:
RuntimeError: Non Expected exception in `/home/vsts/work/1/s/doc/io.rst` line 388
##[error]The operation was canceled.

@dcherian
Copy link
Contributor

Thanks @ulijh. The error here is ModuleNotFoundError: No module named 'h5netcdf' which means that we need to add h5netcdf to the environment yaml file: doc/environment.yaml

@ulijh
Copy link
Contributor Author

ulijh commented Aug 22, 2019

Thanks @dcherian and @shoyer for the review and advice! It seems the docs build and the checks are fine now. One more thing, as this is my first PR to xarray: Should I merge/rebase master into the allow_invalid_netcdf branch, or will you guys take it from this point?

@dcherian
Copy link
Contributor

dcherian commented Aug 22, 2019

We squash merge and the button's green so you're all good.

Thank you for your contribution 👏

@dcherian dcherian merged commit 76d4a67 into pydata:master Aug 22, 2019
@ulijh ulijh deleted the allow_invalid_netcdf branch August 22, 2019 20:20
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.

Unable to set invalid_netcdf=True
3 participants