Disallow explicit indices in coordinates

[maarten-ic]

Fixes #157

As discussed in the progress meeting this morning. Note that this makes some existing coordinates invalid (hence the failing CI):

• https://imas-data-dictionary.readthedocs.io/en/latest/generated/ids/camera_ir.html#camera_ir-channel-camera-calibration-transmission_barrel
• https://imas-data-dictionary.readthedocs.io/en/latest/generated/ids/camera_ir.html#camera_ir-channel-camera-calibration-transmission_mirror
• https://imas-data-dictionary.readthedocs.io/en/latest/generated/ids/camera_ir.html#camera_ir-channel-camera-calibration-transmission_window

---

📚 Documentation preview 📚: https://imas-data-dictionary--179.org.readthedocs.build/en/179/

[github-actions]

Diff of generated Data Dictionary XML file

[imbeauf]

Mentioning an explicit index in an AoS may be linked to the assumption that the AoS children have always the same size, as in the camera_ir example : the size of camera frames doesn't change with time. We should be able to make this assumption explicit and to allow referring to the first index only. Otherwise we would have to repeat the calibration data for each time slice in the camera_ir IDS

[olivhoenen]

Mentioning an explicit index in an AoS may be linked to the assumption that the AoS children have always the same size, as in the camera_ir example : the size of camera frames doesn't change with time. We should be able to make this assumption explicit and to allow referring to the first index only. Otherwise we would have to repeat the calibration data for each time slice in the camera_ir IDS

So what is the next step?
Note that I would prefer to not mention anywhere in the DD a valued index of an AoS or array (even if we have decided to follow 1-based indexing for an idspath, it would be better to avoid it altogether as this is going to generate some bugs for sure in user code).
This camera_ir is the only case in all the DD with such index.

[imbeauf]

I am not sure why (1) is problematic. Does it help if we indicate for the coordinate: frame(:)/apparent_temperature ?
And we explain in the definition of frame that the children must all have the same size ?

[olivhoenen]

I am not sure why (1) is problematic.

Could be error prone if C++/Python code users are putting this 1 index in their code (and again it's the only place in the entire DD where an index value is explicitly given).

Does it help if we indicate for the coordinate: frame(:)/apparent_temperature ? And we explain in the definition of frame that the children must all have the same size ?

That would be ok I guess ,to be checked with the validate functions. And note that we can't say that all child of frame can have the same size, as I guess that region_of_interest may typically change with time (while filter is probably fixed size, as apparent_temperature and integration_time, which already mention this assumption)

[maarten-ic]

Does it help if we indicate for the coordinate: frame(:)/apparent_temperature ? And we explain in the definition of frame that the children must all have the same size ?

That would be ok I guess ,to be checked with the validate functions.

The validation logic should be able to handle this (no validation is performed for this case, as discussed in IMAS-4675). IMAS-Python handles this case here: https://github.com/iterorganization/IMAS-Python/blob/develop/imas/ids_coordinates.py#L374-L381

And note that we can't say that all child of frame can have the same size, as I guess that region_of_interest may typically change with time (while filter is probably fixed size, as apparent_temperature and integration_time, which already mention this assumption)

The number of pixels is not time-dependent in this IDS (link to docs), so all frames must have the same number of pixels, no?

[imbeauf]

At our last progress meeting, we have agreed to use the syntax frame(:) to note that any index will work, in case the coordinate is within an AoS but always has the same size (for all AoS indices).
I have modified camera_ir accordingly, restarting from Maarten's branch but creating my own branch in my own repository since I couldn't commit on his branch directly.
See #198, which replaces the current PR