kineticstoolkit / kineticstoolkit

An Open-Source Python Package to Facilitate Research in Biomechanics
https://kineticstoolkit.uqam.ca
Apache License 2.0
81 stars 8 forks source link

Investigate adding a generic `info` property to TimeSeries #216

Open felixchenier opened 10 months ago

felixchenier commented 10 months ago

As proposed by @opherdonchin in #215 it could be practical to add a generic info property to TimeSeries, for information on the TimeSeries itself, for example the subject ID, or original metadata from the recording instrument.

felixchenier commented 9 months ago

Note: this could be an occasion to remove the "almost useless" time_info attribute, which always contains only a "Unit" field. This way, we would have:

And TimeSeries.time_info["Unit"] would be migrated to TimeSeries.info["TimeUnit"]

This is just an idea. In any case, hidden properties time_info that map to the new info attribute must be created with an eventual deprecation warning so that calls to time-info continue working.

Important: what do we do for loading TimeSeries where people could have added things to the time_info property?

opherdonchin commented 9 months ago

This makes sense to me. I think the obvious way to load data with a time_info property is to default to inserting its value in the TimeSeries.info["TimeUnit"] property, but to give a warning and provide a flag (keep_time_info) to the load routine which retains the old structure in a hidden property.

Actually, it would also be possible to just have both. That is, the property is copied into TimeSeries.info["TimeUnit"] but also maintained as a hidden time_info property. A warning is issued, but that's all.

Does that seem reasonable?