mne-python icon indicating copy to clipboard operation
mne-python copied to clipboard

[MRG] Improves functionality of ANT Neuro reader

Open proloyd opened this issue 1 year ago • 5 comments

Improves functionality added in #12792

What does this implement/fix?

  • Adds following meta information in RawANT.info:
    • 'subject_info' ('his_id', 'first_name', 'sex', 'birthday')
    • 'device_info' ('type', 'model', 'serial', 'site')
    • 'meas_date'
  • Enables lazy data reading (i.e.preload=False argument) in read_raw_ant()

Additional information

This requires bumping up the antio version to 0.3.0.dev0.

proloyd avatar Aug 26 '24 05:08 proloyd

@mscheltienne here is the MNE pull request. Could you please review the test functions? Thanks!

proloyd avatar Aug 26 '24 05:08 proloyd

@mscheltienne how shall we handle different datasets for testing? If I use pytest.mark.parametrize("dataset", ["ca_208", "andy_101"]) and never mention andy_101 explicitly, I see the following vulture error: mne/io/ant/tests/test_ant.py:74: unused function 'andy_101' (60% confidence) Does it anything to do with removal of testing.requires_testing_data decorator?

proloyd avatar Aug 26 '24 17:08 proloyd

I'll cut the 0.3.0 release tomorrow and will have a look at this PR then, I'm not sure I understand this last comment from my phone only.

mscheltienne avatar Aug 26 '24 17:08 mscheltienne

@mscheltienne how shall we handle different datasets for testing? If I use pytest.mark.parametrize("dataset", ["ca_208", "andy_101"]) and never mention andy_101 explicitly, I see the following vulture error: mne/io/ant/tests/test_ant.py:74: unused function 'andy_101' (60% confidence) Does it anything to do with removal of testing.requires_testing_data decorator?

I haven't looked closely as to why you need to define those extra funcs (ca_208 and andy_101) and whether they specifically need to be in the test file, but FYI Vulture is finicky. It might be happier if you moved those definitions into our conftest.py file. If that doesn't work, you can add the func name to tools/vulture_allowlist.py

drammock avatar Aug 26 '24 17:08 drammock

I pushed a couple of changes, LGTM. If vulture complies, then the function should be added to tools/vulture_allowlist.py (done in the last commit). Let me know if that looks good to you!

mscheltienne avatar Aug 28 '24 15:08 mscheltienne