issue_comments

It sounds like we should control this in xarray to ensure consistent behavior.

@dnowacki-usgs - you've made a good point. At least for the netCDF4 backend, this seems to work out of the box with None/False. Can someone check that this works for the scipy/h5netcdf backends?

https://github.com/Unidata/netcdf4-python/blob/366debfff8b0bc53999c9e1ce9f4818bf7cf079a/netCDF4/_netCDF4.pyx#L3455-L3457

Agreed, None is probably better. There is no such thing as a "null" dtype.

On Thu, Sep 28, 2017 at 1:10 PM Joe Hamman notifications@github.com wrote:

I actually think we should use None as the _FillValue sentinel value. We do (sort of) support boolean arrays (#849 https://github.com/pydata/xarray/pull/849).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/issues/1598#issuecomment-332950001, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1pR8YDZ9-Sw_cm4ckTI4XAV45UlOks5sm_0mgaJpZM4Pnox9 .

I actually think we should use None as the _FillValue sentinel value. We do (sort of) support boolean arrays (https://github.com/pydata/xarray/pull/849).

There is also the philosophical problem of fill values for coordinate variables.

Indeed, this is prohibited by CF conventions -- but xarray (like pandas) takes a more flexible approach here, allowing for missing values for all variables.

You can already specify an explicit choice for _FillValue, e.g., ds.to_netcdf(..., encoding={'my_variable': {'_FillValue': 1e35}}). Allowing {'_FillValue': False} to indicate that _FillValue should not be included would be a simple, easy fix, so we should probably do that regardless.

(There is no need worry about False conflicting with a legitimate fill value since netCDF does not have a boolean dtype.)

cc @thenaomig @laliberte

There are at least two ways to fix this: 1. Support a flag of some sort in encoding (e.g., _FillValue = False) to indicate that fill value shouldn't be added. This would be easy to add, but is somewhat inelegant. 2. Check for the presence of NaNs before setting _FillValue = NaN. This would be easy to add for dimension coordinates because they are already guaranteed to be in memory, but could cause performance trouble if any inputs are loaded as dask arrays. I don't know a satisfactory way to handle dask arrays with our current design, since we don't want to add another pass over the data to check for NaNs. I suppose one option would be to refactor our backend classes to write data before writing attributes and then make some sort of dask array operation that checks for NaNs as the data is written. But I'm not even sure this would work with the standard dask task schedulers.