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

_FillValue and missing_value not allowed for the same variable #1749

Closed
aluhamaa opened this issue Nov 30, 2017 · 4 comments · Fixed by #2016
Closed

_FillValue and missing_value not allowed for the same variable #1749

aluhamaa opened this issue Nov 30, 2017 · 4 comments · Fixed by #2016

Comments

@aluhamaa
Copy link

aluhamaa commented Nov 30, 2017

Problem description

Currently opening the file which has a variable with both _FillValue and missing_value defined, results in a ValueError "'Discovered conflicting _FillValue and missing_value...", which can be overridden by setting decode_cf=False

Code in xarray/conventions.py have comments that missing_value is deprecated, however this seems not to be true anymore, see ticket #67 in http://cfconventions.org/cf-conventions/cf-conventions.html

Also, see description of meanings of _FillValue and missing_value here:
Unidata/netcdf4-python#94
"Note that missing_value and _FillValue are very different. _FillValue is used by the C library to fill in values in variables that have not yet been written to. missing_value is meant to represent values set by the user to represent data points that do not exist."

Expected Output

Accept both _FillValue and missing_value and if both present, mask output only for _FillValue

Output of xr.show_versions()

0.9.6

@shoyer
Copy link
Member

shoyer commented Dec 1, 2017

I can certainly see a case for taking a more relaxed approach to _FillValue and missing_value, e.g., masking on both rather than raising an error. I'm not sure that it's actually more user friendly to ignore missing_value for masking.

This part of xarray is very old (added in #245), before almost anyone outside Climate Corporation was using it. So I'm certainly open to revisiting this, but let's make sure we get the decision right this time.

Does anyone else with lots of experience with different netCDF files have opinions here? Maybe @dopplershift?

@aluhamaa
Copy link
Author

aluhamaa commented Dec 1, 2017

I'm fine with masking both missing_value and _FillValue.

@dopplershift
Copy link
Contributor

Given the definitions of these two, as I understand them, I think it makes sense to mask both values.

@jhamman
Copy link
Member

jhamman commented Dec 1, 2017

A practical downside to masking both values is that we won't be able to round-trip the two part fill/missing value. Once everything is a NaN, that's all we get. That said, I'm personally fine with masking both.

dopplershift added a commit to dopplershift/xarray that referenced this issue Mar 26, 2018
The CF standard permits both values, and them to have different values,
so we should not be treating this as an error--just mask out all of
them.
dopplershift added a commit to dopplershift/xarray that referenced this issue Mar 28, 2018
The CF standard permits both values, and them to have different values,
so we should not be treating this as an error--just mask out all of
them.
dopplershift added a commit to dopplershift/xarray that referenced this issue Mar 28, 2018
The CF standard permits both values, and them to have different values,
so we should not be treating this as an error--just mask out all of
them.
dopplershift added a commit to dopplershift/xarray that referenced this issue Mar 28, 2018
The CF standard permits both values, and them to have different values,
so we should not be treating this as an error--just mask out all of
them.
dopplershift added a commit to dopplershift/xarray that referenced this issue Mar 28, 2018
The CF standard permits both values, and them to have different values,
so we should not be treating this as an error--just mask out all of
them.
shoyer pushed a commit that referenced this issue Mar 31, 2018
* Allow _FillValue and missing_value to differ (Fixes #1749)

The CF standard permits both values, and them to have different values,
so we should not be treating this as an error--just mask out all of
them.

* Add whats-new entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants