Do not store zero time increment values in coordinateTransforms#281
Conversation
While the OME model allows any floating point value for the time increment, a 0 value in the scale transformation in the coordinateTransforms can create confusion when interpreted as a downsampling factor between resolutions
melissalinkert
left a comment
melissalinkert
left a comment
There was a problem hiding this comment.
Makes sense to me (nitpicky comment aside 😄).
Separately, I am a little surprised that setting 0 was allowed, and wonder why TimeIncrement is xsd:float in OME-XML schema in the first place. All of the other units are PositiveFloat, and having TimeIncrement as PositiveFloat as well would have avoided this problem in the first place. I don't remember and can't easily find a reference to any discussion, but it would be interesting to know if there is a use case for non-positive TimeIncrement values that might have motivated that type difference. Or maybe it's just that time is the only non-physical dimension with units? In any case, that's outside the scope of this pull request.
| return meta.getPixelsTimeIncrement(seriesIndex); | ||
| Quantity timeIncrement = meta.getPixelsTimeIncrement(seriesIndex); | ||
| if (timeIncrement != null && timeIncrement.value().doubleValue() > 0) { | ||
| return meta.getPixelsTimeIncrement(seriesIndex); |
There was a problem hiding this comment.
Any reason not to just return timeIncrement here instead of calling the getter again?
2 participants
Fixes #280
While the OME model allows any floating point value for the time increment, a 0 value in the scale transformation in the coordinateTransforms can create confusion when interpreted as a downsampling factor between resolutions
Using the sample file provided in the corresponding issue,
bioformats2rawshould produce an OME-Zarr dataset where thescaletransformation includes a0.0value for the time axis.With this change included, PixelsTimeIncrement values of zero should be ignored and the
scalearrays should use the default1.0