Skip to content

Conversation

@proloyd
Copy link
Contributor

@proloyd proloyd commented Aug 26, 2024

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
Copy link
Contributor Author

proloyd commented Aug 26, 2024

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

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me but I'll let @mscheltienne comment!

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@proloyd
Copy link
Contributor Author

proloyd commented Aug 26, 2024

@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?

@mscheltienne
Copy link
Member

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.

@drammock
Copy link
Member

@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

@mscheltienne mscheltienne requested a review from drammock as a code owner August 28, 2024 15:57
@mscheltienne
Copy link
Member

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 mscheltienne enabled auto-merge (squash) August 29, 2024 08:48
@mscheltienne mscheltienne changed the title Improves functionality of ANT Neuro reader [MRG] Improves functionality of ANT Neuro reader Aug 29, 2024
@mscheltienne mscheltienne merged commit af19be0 into mne-tools:main Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants