Error when storing IEEE NaN & Infinity in FITS header keywords - #954

I almost closed this as a duplicate of #873, but I'm going to leave it open because this is really sort of the flip side to #873. Where #873 complained that pyfits reads 'inf' and 'nan' in keywords as invalid, this shows that's possible to assign inf and nan as keyword values, but that the behavior is buggy.

That's a interesting data point that IDL allows this. Now I'm even more tempted to say that it should be allowed, and the fact that the FITS standard does not allow this should be considered an oversight. Though Phil Hodge seems to think it was for some reason intentional. Might as well ask the FITS mailing list...

For what it's worth, fitsverify reports a header like this as invalid.

"like this" meaning including NAN.0, or including the IDL-style NAN?

I agree that the FITS standard does not speak to this usage. In practice this does feel like an oversight. The relevant use case which raised this issue is a keyword that tracks some environmental condition, and if the sensor is offline for whatever reason the relevant software API reports NaN to indicate a missing value. There are various ways to special case handling that in our output software, but just writing the NaN into the keyword would be far and away the simplest & cleanest conceptually. Versus for instance writing the string 'NAN' in quotes in the header, which is going to depend on the reader being smart enough to cast that from a string to a float. (And if we're going to start making specific assumptions of reader programs like that, we might as well just go ahead and use a nonstandard feature like writing NANs to keywords directly!)

The FITS standard does explicitly support IEEE NaN as a missing value indicator in image arrays and tabular data, so it would seem to make sense for it to serve that purpose in header keywords as well.

"like this" = both; that is, neither is valid. What pyfits is doing currently seems to be it trying to do its best with undefined behavior and coming up with something plain silly 😥

As you wrote, it supports IEEE floating point data for array data, so I feel like this is an unfortunate oversight. Hopefully this can be fixed in the standard as well as in CFITSIO (because currently anything that uses CFITSIO will choke on this as well--I'm surprised IDL allows it (not so much)).

Hmm... I'm tempted to say we should just follow the IDL form, given plenty of people are probably used to that behavior, but maybe raise a warning? Something like "NaNs and infs are not officially supported by FITS"? I think warnings about NaNs and infs are generally so common as to be ignored by the average user, anyway :)

I might go that route. If it were at least a registered convention I could support it without even a warning. But to support this there should definitely be a warning, and an option to disable the ability completely at least on writing, as some software (anything using CFITSIO for example) will fail to read files like this.

+1 to using the IDL convention - NAN.0 and INF.0 always seemed a bit strange to me.

Well the NAN.0 and INF.0 is purely by accident. It's not a "convention"--it's just pyfits stumbling on an undefined case. But I can't just blithely say, "okay, you are free to write files with this IDL convention" because lots of standard FITS readers will blow up on that.

I sent an e-mail to the FITS bits mailing list but haven't heard any response yet. Maybe I'll just bug Bill Pence directly.

Did you get a chance to raise this question with Perry or Bob Hanish?

As I wrote in #873 I'm going to go ahead an "fix" this issue by disallowing assigning NAN and INF as header values, since right now the result of that is undefined behavior. Later I will add an option to pyfits to enable them as valid header values.

I've attached a fix that, as I said, disables this for now. Assuming the tests pass I'll then merge it for 0.2.2. In the meantime I'll work on a separate fix to enable this optionally. That will be available hopefully in 0.3.

👍