User defined groups within metaDataTags

[samuelpowell]

The specification permits user defined records within the metaDataTags. Rather than clutter this with a lot of entries, we would like to collect these under a group, e.g., /nirs(i)/metaDataTags/lumo/... . This approach currently fails validation, not explicitly, but because I think the Python code is iterating over what it expects to be datasets.

Irrespective of the validator, I'm not clear if this approach is in spec or not. Any guidance appreciated.

[rob-luke]

My reading of the spec is that the use of another level of grouping is not explicitly denied.

However, as an fNIRS software maintainer I would not encourage this. First, I don't think that average users will be looking at /nirs(i)/metaDataTags/ manually, so clutter here will not bother anyone. Instead, if everything is flat it will be easier to loop over. Secondly, it will make writing readers more difficult, as there is now vendor specific styles of SNIRF to read. Third, how many layers deep of groups do we allow?

Instead, I suggest crowding the /nirs(i)/metaDataTags group with as much information as you wish. If you can use generic names, rather than lumo specific that would be even better. This would allow other manafacturers to reuse the same tags and the readers can be more generic. For example, rather than /nirs(i)/metaDataTags/lumo/led_power or /nirs(i)/metaDataTags/lumo_led_power, just use /nirs(i)/metaDataTags/led_power. And if you think anyone else will use a similar name (perhaps power_led in this example), add your preferred term to the suggested metadata tags table in the spec to encourage consistency across files.

But this is just a suggestion, I think the spec is unclear on this.

[samuelpowell]

@rob-luke yes I take your point regarding vendor neutrality.

We will proceed by including the additional metadata that may be of interest to the user as individual entries, as per the intent of the specification.

The optional extended metadata which is mostly of interest for debug / developer purposes, will be moved to a new root group. This optional data is not enabled by default in our export package (https://github.com/Gowerlabs/lumomat), and when included triggers an appropriate warning in the validator.

I will leave this issue open as:

• the validator should tolerate (but issue warnings or fatal errors) when it finds additional groups under nirs{i}/metaDataTags
• it might be prudent to make the intent explicit in the specification

[sstucker]

Opened a PR which fixes this in the laziest way possible by banning the use of sub groups: #114

I agree wholeheartedly that a flat metaDataTags group is preferable from a software maintenance perspective. I am anxious about the fact that the spec is set to diverge from the original definitions of its organization (#107, #103). While HDF5 is very flexible and allows human-readable refactorings of all sorts, the code we write to support SNIRF is not so forgiving.

In the case of issues 107 and 103 I agree a change must be made. but this case is not so compelling

[samuelpowell]

@sstucker no problem. Reasoning is sound and we can work with this. If time permits is may be desirable to have the validator throw an appropriate error when this is violated, it is currently a crash as it assumes it will find a dataset, not a group.

[sstucker]

@sstucker no problem. Reasoning is sound and we can work with this. If time permits is may be desirable to have the validator throw an appropriate error when this is violated, it is currently a crash as it assumes it will find a dataset, not a group.

I need to make the pysnirf2 validator more robust in general... raising SnirfFormatError and catching it in the validator rather than crashing