FILLVAL Bug?

[bryan-harter]

(From Greg Lucas):

it looks like cdflib defaults to the recommended ISTP FILLVALs based upon the datatype. So even if you specify a FILLVAL in the attributes, it will still use the default. Is this expected behavior?

cdflib/cdflib/xarray/xarray_to_cdf.py

Lines 54 to 72 in 6d3f966

	DATATYPE_FILLVALS = {
	1: np.int8(-128),
	2: np.int16(-32768),
	4: np.int32(-2147483648),
	8: np.int64(-9223372036854775808),
	11: np.uint8(255),
	12: np.uint16(65535),
	14: np.uint32(4294967295),
	21: np.float32(-1e31),
	22: np.float64(-1e31),
	31: np.float64(-1e31),
	32: np.complex128(complex(-1e31, -1e31)),
	33: np.datetime64(-9223372036854775808, "ns"),
	41: np.int8(-128),
	44: np.float32(-1e31),
	45: np.float64(-1e31),
	51: np.str_(" "),
	52: np.str_(" "),
	}

I ran into this because I'm loading a dataproduct that has a different FILLVAL specified and that isn't getting turned into nan's like I expected it to.

[tmplummer]

Related to this. I discovered that when a floating point xr.DataArray, variable, where variable.attrs["FILLVAL"] == np.nan, the CDF contains FILLVAL = 0.0. I narrowed this down to the call to np.nan_to_num() in xarray_to_cdf.py on line 224. It looks like the second argument needs to be passed in as a keyword.

For example, try the following:

print(np.nan_to_num(np.nan, -1e31))
print(np.nan_to_num(np.nan, nan=-1e31))

the result is:

np.float64(0.0)
np.float64(-1e+31)

This is fairly high priority to get fixed as it is affecting IMAP processing. I have a workaround for the issue it is causing for the Hi instrument.

[bryan-harter]

I just pushed that change into a new version, 1.3.8. Thanks a lot for tracking that down, it definitely saved me an hour or two if I had to do it myself.

That said, I think the original bug report is a different issue, though it maybe have been related if @greglucas was noticing weird behavior. The way xarray_to_cdf is written right now, it forces all NaNs to become the ISTP compliant FILLVAL and doesn't look at all what the user has set as the attribute FILLVAL. I believe my logic was that we are trying to create ISTP compliant files, and using a different fillval wouldn't be ISTP compliant. Is there still a use case @greglucas for having a custom FILLVALs, rather than the ISTP defined ones? If there is for IMAP, I can leave it up at P0 (high priority). If it isn't crucial, I can maybe re-classify this as a Feature Request and move it down to P1.

[greglucas]

That is a good question if that is required for ISTP compliance or not. I think just having a FILLVAL is the requirement, but it isn't the clearest wording to me.
https://spdf.gsfc.nasa.gov/istp_guide/vattributes.html#FILLVAL

I think those are "standard" fillvals, but not the only ones that can be used. I think it would be nice to follow the user's request if they specify one, and then fallback to the standard one if it is unspecified. This isn't crucial for IMAP, so feel free to move it down a level.

[bryan-harter]

Yeah I agree it isn't super clear, I'm curious what the ISTP checker would do with a non-standard FILLVAL. If it does wind up giving a warning or something, cdflib should probably check all FILLVALs it sees and make sure they are the stand ones. Otherwise, I'll just have cdflib check the user-supplied FILLVAL first.

Either way it should be very quick code changes, the "hard" part is just verifying what it should do!