neurogeriatricskiel / NGMT

Python based toolbox for processing motion data
https://neurogeriatricskiel.github.io/NGMT/
Other
6 stars 1 forks source link

Update needed for handling non-existent tracked points in importers #86

Closed masoudabedinifar closed 1 month ago

masoudabedinifar commented 1 month ago

Dear @rmndrs89 and @JuliusWelzel,

Following our discussion with @rmndrs89, it's been brought to our attention that there's a need to address the case where the input tracked_points do not exist in the list when using importers. Currently, there's a suggestion to issue a warning and return empty pandas.DataFrame for both data and channels, as drafted here. We may need to discuss alternative suggestions on how to handle this case.

Best, Masoud.

JuliusWelzel commented 1 month ago

According with BIDS, if you specify data you have to provide a label for a tracked point: https://bids-specification.readthedocs.io/en/stable/modality-specific-files/motion.html#channels-description-_channelstsv

I am happy if we return a warning, not an error saying we recommend specifying a label for the tracked point or points of that data that has been imported.

rmndrs89 commented 1 month ago

Hi @JuliusWelzel ,

I think you are absolutely right, but that was not what @masoudabedinifar hinted at ;-)

Suppose that you have data from "LowerBack", "LeftFoot", "RightFoot".

data, channels = mydata.load_recording(file_name, tracked_points=["LowerBack"])

should return the data and channels for the "LowerBack"-worn IMU.

But what happens when we do:

data, channels = mydata.load_recording(file_name, tracked_points=["Pelvis"])

Should we raise a ValueError saying that "Pelvis" tracked point does not exist, or raise a warning and still return an empty dataframe?

What if we do?

data, channels = mydata.load_recording(file_name, tracked_points=["Pelvis", "LeftFoot", "RightFoot"])

Should we raise a ValueError saying that "Pelvis" tracked point does not exist, or raise a warning and still return a dataframe for the valid tracked points (here, "LeftFoot", "RightFoot")?

What would you suggest? (My gut feeling tells me a ValueError is probably the cleanest way.)

JuliusWelzel commented 1 month ago

Sorry for the misunderstanding. Tracked point in the importers I wrote is used to label the data you are loading as it has no label in the raw file.

For the cases @rmndrs89 described, I would return an Error when only "invalid" tracked points are used, and a Warning if the list contains one or more invalid tracked points.

What do you think?

masoudabedinifar commented 1 month ago

Based on our discussion, I updated the APDM Mobility Lab importer to raise an error when invalid tracked points are entered. You may see the changes in this commit in the sense-park-data-importer branch as: Track invalid tracked points.

Once you approve, we may close the issue.

Kind regard, Masoud.

JuliusWelzel commented 1 month ago

fixed in https://github.com/neurogeriatricskiel/NGMT/pull/87