Skip to content

write subject ID to ["his_id"], not ["first_name"]#546

Merged
larsoner merged 7 commits intomne-tools:mainfrom
aaronjnewman:subj_id-fix
Apr 3, 2024
Merged

write subject ID to ["his_id"], not ["first_name"]#546
larsoner merged 7 commits intomne-tools:mainfrom
aaronjnewman:subj_id-fix

Conversation

@aaronjnewman
Copy link
Copy Markdown
Contributor

Reference issue

Fixes #544

What does this implement/fix?

write subject ID to ["subject_info"]["his_id"], not ["subject_info"]["first_name"]. This makes the code more consistent with BIDS specifications, and fixes error generated by write_raw_snirf() described in #544

@rob-luke
Copy link
Copy Markdown
Member

Thanks @aaronjnewman. My first gut feeling is that this makes sense. I just enabled the CI to run the tests. Don't worry if the tests fail, we will work with you to fix any unrelated errors and understand why anything is failing due to the change.

@larsoner
Copy link
Copy Markdown
Member

I think you need to at least adjust this test:

mne_nirs/io/snirf/tests/test_snirf.py:79: in test_snirf_write_raw
    assert diffs == ""
E   assert "\n['subject_info']['first_name'] value mismatch (NIRX_Test, NIRX)" == ''
E     
E     + 
E     + ['subject_info']['first_name'] value mismatch (NIRX_Test, NIRX)

to check his_id now instead of first_name

@aaronjnewman
Copy link
Copy Markdown
Contributor Author

I may have to defer to @rob-luke or others on this one - I'm not familiar with CI and not sure where this originates. I don't see any instance of first_name anywhere in the repository since my change was made.

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Apr 2, 2024

Pushed a commit that should make sense alongside mne-tools/mne-python#12526. We should merge that then restart CIs here then things should be green 🤞

@aaronjnewman feel free to look to see if the two PRs make sense to you

@aaronjnewman
Copy link
Copy Markdown
Contributor Author

@larsoner thanks - like many things, it's clear when I see it, but generating it myself would be a whole 'nother thing :) Appreciate the quick fix!

@larsoner larsoner merged commit 3c5fb56 into mne-tools:main Apr 3, 2024
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Apr 3, 2024

Thanks @aaronjnewman

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

subject ID stored in first_name field, but should be his_id field for MNE-BIDS compatibility

3 participants