NaN and Infinity number values cause write to fail

[richard-viney]

dcmjs@0.29.0 is able to read NaN and Infinity values from a DICOM, but isn't able to write them back out.

Attempting to do so triggers the "Not a number" exception in this function from BufferStream.js:

function toFloat(val) {
    if (isNaN(val)) {
        throw new Error("Not a number: " + val);
    } else if (typeof val == "string") {
        return parseFloat(val);
    } else return val;
}

The expected result is that numeric tags with NaN or Infinity values are written back out. Verifying with pydicom showed that it reads and writes these values.

[pieper]

There are some other places where converting to native times causes differences due to rounding errors, so we thought about keeping the original value in a _original field or similar so that this kind of issue won't lead to a validation error on round trips. Otherwise I'm not sure we want NaNs in the fields that we plan to compute on - a goal of the normalizing steps is to handle all the special cases so that application code isn't littered with if statements. Since I don't think the standard allows NaN or Infinity so it's fair to reject them but I understand the goal of dealing well with nonstandard data.

[richard-viney]

Makes sense, however I don't believe this is non-standard data or a special case. We've just seen it in the real world coming from a GE Vivid S70 v201.102.0 Ultrasound machine.

It appears that NaN values are sometimes put into tags with a VR of FD (double float) that are required by the spec, but for which the machine doesn't have a useful/sensible/legitimate value for. So they put NaN to fill the gap. This seems reasonable, and AFAICT is valid according to the DICOM spec.

The FD VR is defined as Double precision binary floating point number represented in IEEE 754:1985 64-bit Floating Point Number Format. (link), and both NaN and infinity values are defined by IEEE 754:1985, so these values are allowed in FD tags. However they're not allowed in other VRs that store numeric values, such as DS (decimal string) and IS (integer string).

Thoughts?

I've just opened a PR that removes the NaN check and adds a couple of tests.

[pieper]

Ah, I see, FD no DS, yes, allowing NaN and Inf makes sense, thanks for the explanation.

[github-actions[bot]]

:tada: This issue has been resolved in version 0.29.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: