Importer should return data not NGMTRecording

[masoudabedinifar]

Dear @rmndrs89 and @JuliusWelzel,

As we're currently working on writing an importer for Sensepark data, I want to ensure its consistency with import_axivity. However, following a recent discussion with @rmndrs89, it's become apparent that the importer should not return NGMTRecording, as initially thought.

To address this, we may discuss how to move forward with this. You can find the relevant section of the code at [import_axivity]

Kind regards, Masoud.

[rmndrs89]

To follow up on this, the main idea, following the BIDS principles, is that a recording may consist of multiple tracking systems. Each tracking system may consist of several tracked points. Now, when we import data from a given sensor system, e.g., Axivity or Mobility Lab, we may argue that it simply returns the data and channels information for that tracking system. In other words, we would get two pandas.DataFrame returned.

P.S.: also it does not make sense to have tracked_points are a function argument and hardcode it in the function body, like here, rather the tracked_points should be of type str or list[str] and should be used to return only the data (and channels information) for those tracked points (sensor locations). We could also make it optional, and set the default to None, making sure that if it is None then the data for all available tracked points is returned.

[JuliusWelzel]

In my understanding each tracking system would return a new NGMTRecording dataclass.

As for the tracked point I agree with @rmndrs89. It even is in the function as an inputargument of type string.

[JuliusWelzel]

like here,

fixed in https://github.com/neurogeriatricskiel/NGMT/commit/0ad8a30d63a9691524fc177099c410ab3eb30649

[JuliusWelzel]

As agreed yesterday, all importers should not return a NGMTRecording as done here, but should only return the information to construct a NGMTRecording: data and channels as dataframes.

Additionally, we need a function inside the NGMTRecording class which can merge two instances of the class. See https://github.com/neurogeriatricskiel/NGMT/issues/85.

[JuliusWelzel]

@rmndrs89, which commits did change this?

[rmndrs89]

Sorry was looking at different branch and at the importer for Mobility Lab data. Who will take care of this? @JuliusWelzel or @masoudabedinifar or @rmndrs89 ?? For me, the only thing that needs to change is the returned values, right?

[JuliusWelzel]

I can change it for the axivity importer and the branch you look at.

[masoudabedinifar]

I changed the return values of APDM mobility lab importer in the "sensepark-data-importer" branch and will complete the APDM Mobility Lab importer by adding error handling for cases where the user chooses the wrong sensor. The updated importer for the APDM Mobility Lab can be found here: [import_mobilityLab]

[JuliusWelzel]

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