issue_comments

Strangely, the Travis-CI build seems to have failed with some errors unrelated to this PR. The two errors are on:

• Python 3.6 with the development version of dask, giving an error of AttributeError: module 'dask.array' has no attribute 'ghost'
• Python 2.7 with cdat, iris and pynio which gives various errors of arrays not being equal for test_to_and_from_cdms2_sgrid and test_to_and_from_cdms2_ugrid

For what it's worth, all of the tests related to this PR passed.

Ok great, I'll make a PR to remove that last chunk - thanks!

I realised that the reason that Travis and AppVeyor weren't running properly was because there were conflicts between this PR and master. I think I've managed to rebase and push properly - but let me know if I've done anything wrong.

Ah ok - that works (not entirely sure why, but it works!)

Oops yes, sorry!

I'm having a bit of an issue with changing the strings to constants (which, by the way, I completely agree is how it should be done). I've defined the constants in backends/api.py and am then trying to import them in core/dataarray.py.

I am doing the import as:

from ..backends import DATAARRAY_NAME, DATAARRAY_VARIABLE

But this gives me the following error:

../../anaconda3/lib/python3.5/site-packages/py/_path/local.py:650: in pyimport __import__(modname) xarray/__init__.py:3: in <module> from .core.extensions import (register_dataarray_accessor, xarray/core/extensions.py:3: in <module> from .dataarray import DataArray xarray/core/dataarray.py:9: in <module> from ..backends import DATAARRAY_NAME, DATAARRAY_VARIABLE xarray/backends/__init__.py:8: in <module> from .netCDF4_ import NetCDF4DataStore xarray/backends/netCDF4_.py:6: in <module> from .. import Variable E ImportError: cannot import name 'Variable'

It seems like my relative import is somehow mucking up another relative import inside the netCDF4 stuff. Do you have any ideas how I can solve this? Am I just doing my imports wrong?

Great, thanks. I think I've done these changes (frustratingly I can't see a way to use *args/*kwargs after using the full argument list in the function definition, so I've had to write it out. Let me know if there is a better way, but I suspect there isn't).

I also prefer an approach that doesn't use context managers: I agree with @darothen's comments about the issues with their use.

Regardless of the exact implementation, I am strongly in favour of this functionality being added to xarray - I can already think of a number of very useful ways I could use it!

Thanks - I've got that working with the close() method, and I did it 'properly' using the BaseDataObject class.

The only failure on here now is the same as #991 - a conda error creating one of the environments.

The Travis builds seem to have all passed except for py27-cdat+pynio which has failed with a conda error that seems to be unrelated to these changes (Error: HTTPError: 403 Client Error: Forbidden for url)

Also, it seems like there might be an issue with not closing the Dataset - but only on Windows (see the AppVeyor build failure at https://ci.appveyor.com/project/jhamman/xarray-injyf/build/1.0.268/job/kr8ss684ijxg55be).

Any ideas how this could be sorted without having to implement .close() for DataArray (or, alternatively, how to go about implementing that, given that I don't think a DataArray contains a reference to the Dataset that it may have come from)

I think I've dealt with all of these.

Refactoring the tests has made them a bit cleaner and safer due to not using mutation - however I still have quite a few repeated lines of code. I've also combined them all into one test with lots of asserts - as otherwise I couldn't keep the helper functions within the test without repeating them loads of times.

Does anything need adding to the docs for this? I'm assuming not as it's just making the code do more checking, it's not adding a feature as such?

Thanks for the useful comments.

I've added some bits to the Sphinx docs in the places that I think they should go - but let me know if I need to add any more.

I think I've dealt with the comments/issues now, but let me know if more needs doing.

Thanks, I'll deal with these modifications.

Also, I'm seeing errors from TravisCI saying ValueError: cannot read or write netCDF files without netCDF4-python or scipy installed - what do I need to do to make sure these tests work on Travis? (They pass fine on my local machine)

Thanks @shoyer - I wasn't actually aware that DataArrays can have a name before they're put into a Dataset (that's one of the things I love about contributing to OSS: you always learn things about the software that you never knew before!). I'll deal with the design issues ASAP.

How would you suggest testing this? I wasn't sure where to put the tests, and what exactly to test? Presumably something about the equivalence between this and saving after manually converting to a dataset, but I wasn't really sure how best to go about it.

I'm not really sure how to work out which one it is a bug in - do you have any advice? Or should I just report it on the dask issue tracker too and see what they think?

Thanks. I might raise an issue on the netCDF4-Python project to see if we can improve the error message.

Would you be happy for me to submit a PR updating the xarray docs to note that you can only export attributes with certain values to a netCDF file? (and possibly, longer-term, we could provide an error inside xarray if you try to save out a file with objects as attributes)

Coming here from #897, it seems that we just need to decide to go with matplotlib's version, and therefore switch round the tuple given as figsize at https://github.com/pydata/xarray/pull/637/commits/400b1ef1d142c3c0dec1ff3c9f3cdfffa2229c10#diff-4ee153eb90c5207a5ed2e2b997bf6ce9R212, and then update the docs to go with it.

Does that make sense?