I think #2298 is what I'm really waiting for and would solve the use cases I listed above. I'll have no trouble using the accessor methods in the time being. Thanks, @shoyer

The only problem I see with this is that actual.acc.altitude must be called before actual.altitude, otherwise it will result in the data_var being used before it is created.

Thanks for this @shoyer.

Thanks, I will gladly exercise patience until then.

1865

It's a simple fix, but it affects my ability to produce CF1.7 netCDF files for distribution of our satellite data. The pull request #1869 is ready to go.

@shoyer, I think everything is in order to apply this bug fix.

@shoyer, do you have an opinion on whether or not to include the scipy fix or leave it out of this PR? Also what is your opinion on the documentation? I feel nothing is needed as with this PR the code will operate as the documentation states.

Now the tests pass (see comment beside the checkbox)

Honestly, I'm not happy with where I put the fix in order to get the tests to pass when using the scipy backend by adding an additional test, but I didn't know where else to put the test. The test to see if the variable would be checked in prepare_variables came before the encoding could be managed in encode_cf_variable in conventions.py

The alternative would be to remove the changes to scipy_.py and just have the test run only for the other engine backends and leave fixing the scipy backend for someone more familiar with it for another issue.

Wait to merge... I'm still getting failed tests using the scipy backend

Unit tests added in 9210dfa45428842c69b8d1835013159b1230c572. The first and the third added tests use the encoding kwarg and fail without b8fd86d5614d28254406d6f9b6d4e37383efc1be applied and pass with b8fd86d5614d28254406d6f9b6d4e37383efc1be applied, so they should catch a regression.

The second added test passes both before and after b8fd86d5614d28254406d6f9b6d4e37383efc1be and was added for completeness as it pairs with the original test_explicitly_omit_fill_value, but for a coordinate.

I still think nothing needs to change in the documentation as this now follows the intent of the design.

I will leave it to you @shoyer or your delegate to address whether anything needs to be stated in the changelog / what's new.

@shoyer, I'll look at the documentation once this is resolved and make a PR.

My apologies for having to re-open the issue, I should have tried the fix before closing.

I get the following error after changing my last line to: ds.to_netcdf('/tmp/test.nc', encoding={'lat': {'_FillValue': None}, 'lon': {'_FillValue': None}}) as suggested.

File "/path/to/python3.6/site-packages/xarray/backends/netCDF4_.py", line 354, in prepare_variable unlimited_dims=unlimited_dims) File "/path/to/python3.6/site-packages/xarray/backends/netCDF4_.py", line 176, in _extract_nc4_variable_encoding ' %r' % (backend, invalid)) ValueError: unexpected encoding parameters for 'netCDF4' backend: ['_FillValue'] Do we just need to append '_FillValue' to the list of ` valid_encodings = set(['zlib', 'complevel', 'fletcher32', 'contiguous', 'chunksizes', 'shuffle'])

Thanks.

Please add the label "CF conventions" to this issue

Thanks for the heads-up @darothen. I browsed the issues to make sure I wasn't duplicating anything, but I must have overlooked the pull request you mentioned. I have used a lambda function in the interim, so it is no issue if this addition waits until your pull-request is merged in.