Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Adding ITAB file reader #12439

Open
robbisg opened this issue Feb 13, 2024 · 2 comments
Open

ENH: Adding ITAB file reader #12439

robbisg opened this issue Feb 13, 2024 · 2 comments
Labels

Comments

@robbisg
Copy link

robbisg commented Feb 13, 2024

Describe the new feature or enhancement

We would like to add the ITAB raw file reader to MNE-Python. @vpizzella made a first draft of the reader and I am prepping it following your guidelines with tests and linting.
I have a bunch of question regarding the management of testing data and tests:

  1. I noticed that data is included in another repo, should I open a PR also in this repo?
  2. Is there a size limit of the files?
  3. Is it better a file that includes all features implemented (and tested) or it is better a set of files?
  4. I used a matlab (fieldtrip) file to test whether the reader reads the same numbers, is it correct or is there any other strategy?

Describe your proposed implementation

A first draft of the implementation can be found here, but it needs linting and so on.

Describe possible alternatives

We followed other readers implementation, so it is similar to other readers and maybe optimal.

Additional context

No response

@robbisg robbisg added the ENH label Feb 13, 2024
Copy link

welcome bot commented Feb 13, 2024

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴

@larsoner
Copy link
Member

I noticed that data is included in another repo, should I open a PR also in this repo?
Is there a size limit of the files?

Yep, PR there first is great. "As small as possible" is the rule of thumb. In practice for most formats we get away with something < 1 MB. Basically ~1sec of data or less is hopefully enough. But if your acquisition system writes like 10s blocks or something, then you'd want 20s of data for example.

Is it better a file that includes all features implemented (and tested) or it is better a set of files?

I would say whatever uses the least disk space while providing proper code/functionality coverage. So for example a file with weird lowpass settings and UTF encoding, and another file with normal lowpass settings and latin-1 encoding would be fine and preferred over 4 files (normal lowpass, weird lowpass, utf8, latin-1).

I used a matlab (fieldtrip) file to test whether the reader reads the same numbers, is it correct or is there any other strategy?

Yes if you have a reference implementation you know/trust is correct then often what we do is have the native format plus a little .mat file containing the data read with FieldTrip or EEGLAB or whatever, then we assert_allclose(raw.get_data(), read_mat(...)['data']) (paraphrasing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants