issue_comments

Yes, I still think that makes sense. I don't like issuing warnings other than for deprecation cycles. On Fri, May 13, 2016 at 5:29 PM Joe Hamman notifications@github.com wrote:

@shoyer https://github.com/shoyer - reading back in this thread (#401 (comment) https://github.com/pydata/xarray/pull/401#issuecomment-96085330) you mentioned moving this fix to conventions.py and writing a dtype: bool attribute for bool typed variables. Do you still think that makes since?

Alternatively, we could just raise a warning when typecasting on write. This is the lazy but less intrusive way of operating.

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/pydata/xarray/pull/401#issuecomment-219188897

@shoyer - reading back in this thread (https://github.com/pydata/xarray/pull/401#issuecomment-96085330) you mentioned moving this fix to conventions.py and writing a dtype: bool attribute for bool typed variables. Do you still think that makes since?

Alternatively, we could just raise a warning when typecasting on write. This is the lazy but less intrusive way of operating.

Karl -- if you or anyone else wants to pick up and finish this PR, that would be awesome. It is very close to being done.

On Fri, May 13, 2016 at 4:30 PM, Karl Lapo notifications@github.com wrote:

I also ran into this problem -- I wanted to save a netcdf with a boolean array. Casting the booleans as ints worked for my application.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/pydata/xarray/pull/401#issuecomment-219183082

@khaeru - do you think you're going to come back to this?

This seems like a pragmatic choice -- but I agree, it's unfortunate that we can't roundtrip this faithfully.

One option would be to add the attribute dtype with the value 'bool' to the netCDF variable, and then teach xray to interpret that attribute when loading data. We already do some pretty similar things to handle datetime64 variables according to the CF conventions. If you're interested in doing this, the right place to put this would be conventions.py: https://github.com/xray/xray/blob/eed58d5cb4274f262e77277bc8fcf394a9fae77c/xray/conventions.py#L632 (for decode, put decoding dtypes after decoding times at the bottom) https://github.com/xray/xray/blob/eed58d5cb4274f262e77277bc8fcf394a9fae77c/xray/conventions.py#L603 (for encoding, write another little helper function and call it at the bottom here)

The other advantage of putting this in conventions is that then it works with all netCDF backends (including scipy and h5netcdf).

Also, this definitely needs tests :). This could be as simple as verifying that it works to roundtrip a Dataset with boolean data to disk and back. Here's an example of what those look like: https://github.com/xray/xray/blob/eed58d5cb4274f262e77277bc8fcf394a9fae77c/xray/test/test_backends.py#L156

How does this handle the round trip conversion from Dataset to netCDF and back to Dataset. Do you now get an array of u1?

Also, it may be good to add a unit test addressing your previous ValueError.