Consider "context" -and perhaps "descriptions"- when reading c3d files

[jorgomezga]

First of all, thanks for the toolbox! It contains a bunch of quite useful tools for viewing, exploring and manipulating kinetic data.

However, It would be great if the toolkit would consider the fields "context" (reader["parameters"]["EVENT"]["CONTEXTS"]["value"]) -and perhaps "descriptions" (reader["parameters"]["EVENT"]["DESCRIPTIONS"]["value"])- when reading c3d files, as they contain valuable information for bilateral event identification like toe_off and heel_strike. In these cases, the field "context" usually records the side of which the event occurs (https://www.c3d.org/HTML/default.htm?turl=Documents%2Fthec3dfileformat.htm). I'll be happy to help if necessary.

Thanks!

[felixchenier]

Hi @jorgomezga

This is a great idea, I don't personally use these fields but I understand that many people do. However, we need to figure out a clean way that fits well with the TimeSeries/TimeSeriesEvent classes.

Currently, TimeSeries events have only two properties: name and time. This makes it very simple to manage with basic functions such as TimeSeries.add_event(time, name), etc. Adding contexts and descriptions would require to either concatenate everything to the event name, which may work well for context:
heel_strike_left where left is the context
but not for description:
heel_strike_left_contact_between_heel_and_ground which is unusable.

The alternative would be to add metadata to events, a bit like we already have with TimeSeries.data_info. In this case, events would have name : str, time : float and event_info : dict[str, str]. This would certainly be the cleanest way to add any arbitrary amount of metadata for an event.

To be coherent with this new feature, we would need to adapt some TimeSeries methods, obviously TimeSeries.add_event() which would allow a new argument for metadata, probably TimeSeries.plot() which should take optional arguments to plot not only the event names but also other metadata (e.g., context), and maybe several others.

Still, I think this would be pretty clean. Would it be something that fits your need and that makes sense for you?

[jorgomezga]

Hi @felixchenier,

Thanks! Your proposal definitely fits my needs.

Adding just context to the concatenation to form something like heel_strike_left sounds like it could do the trick (instead of dealing with both context and description). Though, I also get the point about not wanting to clutter up the TimeSeriesEvent class with a bunch of extra stuff.

Honestly, either option works for me.

[felixchenier]

@jorgomezga I'm happy that both solutions work for you because the more I think of it, the less I want to mess with the TimeSeriesEvent class. If only the context work, then I think I may have a good, configurable, and backward-compatible idea for ktk.read_c3d(). I would add an optional parameter to it, which would be something like event_name_format : str which is a string with placeholders such as {{name}} and {{context}}.

Its default value would be "{{name}}". Which would mean that event names would be simply based on the "NAME" parameter of the C3D. For example: heel_strike. We could customize it, for example {{name}} ({{context}}) which would generates event names such as heel_strike (left). Or any other string to the taste of the user, such as {{context}_{{name}} which would generate left_heel_strike.

I'm creating a feature request out of it and I (or you 😄 ) could start working on it. For the implementation of it, I think of a very simple thing such as

event_name = (read the event name from the c3d dict)
event_context = (read the event context from the c3d dict, empty string if no context)

event.name = event_name_format.replace("{{name}}", event_name).replace("{{context}}", event_context)

We would also need to check if the current test files have a context field in it to build the unit test.

Please feel free to add any idea in the feature request: #194

[felixchenier]

Hi @jorgomezga

This feature is now available using the "include_event_context" option of ktk.read_c3d(), currently in master. It will be available in the next release.

[jorgomezga]

I'm really sorry for1 not reaching out sooner. I just wanted to say a big thank you!! everything's working perfectly, and I truly appreciate all your hard work!

[felixchenier]

Tx for the comment, this it much appreciated :-)