Dataclasses | RecordingData | Why do i need to define `ch_names`

[rmndrs89]

Hey guys,

I am still having troubles understanding our complex dataclass structures. Now I have adopted the importer for the Keep Control dataset, but when I was testing it, I run into an issue related to: https://github.com/neurogeriatricskiel/NGMT/blob/28c6d94c6dc4203ba8a15795dff465994a26d31b/ngmt/utils/ngmt_data_classes.py#L143

Why is there a function that checks https://github.com/neurogeriatricskiel/NGMT/blob/28c6d94c6dc4203ba8a15795dff465994a26d31b/ngmt/utils/ngmt_data_classes.py#L152?

The error I got is:

Exception has occurred: TypeError
object of type 'NoneType' has no len()
  File "/home/robbin/Projects/NGMT/ngmt/utils/ngmt_data_classes.py", line 152, in __post_init__
    if len(self.ch_names) != self.data.shape[1]:
  File "/home/robbin/Projects/NGMT/ngmt/datasets/keepcontrol.py", line 86, in load_file
    recording_data = RecordingData(
  File "/home/robbin/Projects/NGMT/test.py", line 32, in main
    motion_data = keepcontrol.load_file(
  File "/home/robbin/Projects/NGMT/test.py", line 41, in <module>
    main()
TypeError: object of type 'NoneType' has no len()

That means that if I do not specify ch_names for the RecordingData, then it is of type NoneType and the comparison cannot be made.

Should we remove types and ch_names as data attributes, and simply derive them from the ChannelData attribute (these should contain the same information anyway)?

[JuliusWelzel]

Thanks, Robbin for the critical feedback. This is valuable, especially when writing examples for people to import their own data.

I should add a function to the postinit process that derives the types and ch_names automatically. I included these fields, as they are close to the MNE implementation. For some future functions, I see some advantages to having them easy to access. Let's discuss this in our next meeting

[JuliusWelzel]

I updates the dataclasses for now, also made adjustments to the importers in https://github.com/neurogeriatricskiel/NGMT/commit/10af5ec4ce0269324d165d3cc50c253680e55689. Please check if it makes sense. I also updated the documentation a little. Let's still discuss tomorrow, if we are happy moving forward with the current implementation. For me, ch_types or ch_names should be a MUST at some point, but if provided once can be fetched from any class to avoid redundancy.