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

Disallow mismatching axes units and types #91

Open
sbesson opened this issue Feb 2, 2022 · 3 comments
Open

Disallow mismatching axes units and types #91

sbesson opened this issue Feb 2, 2022 · 3 comments

Comments

@sbesson
Copy link
Member

sbesson commented Feb 2, 2022

See discussion at #87 (comment)

Units were introduced in #57 at a SHOULD level of requirement, primarily for extensibility reasons as well as support for axes with arbitrary units. A large part of the discussion about units revolved about the unit validation and the usage of udunits2 as current reference library.

The following axes dictionary is valid according with the current draft version of the specification but is not recommended as the unit is not part of the subset of recommended time units:

    {
        "name": "t",
        "type": "time",
        "unit": "micrometer"
    }

My opinion is that this (potentially corner) case this should be flagged as invalid by the specification.

Using the current specification, I see two immediate ways to enforce this constraint:

  • either tighten the constraints around the units available for the space and time axes types i.e. rewrite

    ngff/latest/index.bs

    Lines 228 to 230 in 18fc37b

    - SHOULD contain the field "unit" to specify the physical unit of this dimension. The value SHOULD be one of the following strings, which are valid units according to UDUNITS-2.
    - Units for "space" axes: 'angstrom', 'attometer', 'centimeter', 'decimeter', 'exameter', 'femtometer', 'foot', 'gigameter', 'hectometer', 'inch', 'kilometer', 'megameter', 'meter', 'micrometer', 'mile', 'millimeter', 'nanometer', 'parsec', 'petameter', 'picometer', 'terameter', 'yard', 'yoctometer', 'yottameter', 'zeptometer', 'zettameter'
    - Units for "time" axes: 'attosecond', 'centisecond', 'day', 'decisecond', 'exasecond', 'femtosecond', 'gigasecond', 'hectosecond', 'hour', 'kilosecond', 'megasecond', 'microsecond', 'millisecond', 'minute', 'nanosecond', 'petasecond', 'picosecond', 'second', 'terasecond', 'yoctosecond', 'yottasecond', 'zeptosecond', 'zettasecond'
    to express that if a unit for the space or time type is specified, it MUST be part of the specified subset
  • or add another constraint expressing the relationship between valid udunits and the axes type i.e. disallow a valid time unit to be associated with a space axis (and inversely)
@constantinpape
Copy link
Contributor

I would be fine with option one (space and time must be in the given list of units).

@sbesson
Copy link
Member Author

sbesson commented Apr 13, 2022

Cross-linking to glencoesoftware/bioformats2raw#144 which brought up the issue of tightening the list of units for space/time and optionally supplementing it by a subset of units defined in the OME specification but not in the NGFF one.

From the discussion esp. glencoesoftware/bioformats2raw#144 (comment), extensibility is probably the primary concept that would speak against maintaining a specified list of allowed spatial/temporal units. In my mind, there are two questions:

  • do we know of real use cases of space/time axis that would require some vocabulary outside the existing list?
  • assuming such a use case, would we consider an update to the specifcation to add new units to a list and how would this happen?

For reference, similar discussions had happened for OME XSD specification in the context of other enumerations such as AcquisitionMode which are much harder to maintain than space/time units due to the technological evolution. The decision was to support the addition of new enums as patch releases of the XSD schemas - see ome/bioformats#2553 for an example. I am in favor of a similar approach for the NGFF specification.

@thewtex
Copy link
Contributor

thewtex commented Apr 16, 2022

For the time units, a few common acquisitions come to mind,

  • Integer unix epoch time
  • A sequential integer time marker where data is acquired at sequential time points, but they are not necessarily on a uniform time scale
  • A date-time, an absolute date-time instead of a relative time

Can these be supported?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants