-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Proj id and proj name access #13261
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
Proj id and proj name access #13261
Conversation
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
for more information, see https://pre-commit.ci
This looks like it's headed in the right direction! If you look at one CI failure, you'll see:
I would suggest to modify Any new change would ideally include some changed test, could you modify Finally, we'll need a |
…rentLM/mne-python into proj_id-and-proj_name-access
for more information, see https://pre-commit.ci
Hi @larsoner, Thanks for your guidance, I'm very new to this. Still a bit confused as to why the test failed. I have added tests and contribution information, hopefully it's all in the right place and the right format. Let me know if not :-) |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
I think I am a bit stuck with this. I fail to understand why mne-python/mne/_fiff/meas_info.py Line 3501 in 68b5523
It would make more sense to me to set it as 0 (as an integer), the same way as subject_info["id"] is anonymized.
|
Agreed, I think you can make this change |
… int when coming from FIFF.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…rentLM/mne-python into proj_id-and-proj_name-access
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, marking for merge when green, thanks in advance @LaurentLM !
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
* upstream/main: (55 commits) doc: fix rendering typo rst docstr (mne-tools#13301) DOC: fix docstrs around layout functions (mne-tools#13300) MAINT: Fix doc build failure due to deprecation (mne-tools#13299) Birthday input cast to datetime.date (mne-tools#13284) DOC: fix missing space, use f-strings, structure->object (mne-tools#13291) [pre-commit.ci] pre-commit autoupdate (mne-tools#13290) ENH: channel_indices_by_type now has an exclude param (mne-tools#13293) Proj id and proj name access (mne-tools#13261) Fix: nearly invisible traces with spatial_colors=True (mne-tools#13286) [pre-commit.ci] pre-commit autoupdate (mne-tools#13283) Bump autofix-ci/action from 551dded8c6cc8a1054039c8bc0b8b48c51dfc6ef to 635ffb0c9798bd160680f18fd73371e355b85f27 in the actions group (mne-tools#13282) fix Maxwell bads filtering (mne-tools#13280) fix actionable linkcheck errors (mne-tools#13273) MAINT: Use radius keyword with PyVista tube (mne-tools#13277) BUG: Fix bug with simulating head pos and BEM (mne-tools#13276) [pre-commit.ci] pre-commit autoupdate (mne-tools#13274) MAINT: Update code credit (mne-tools#13267) Annotations extras (mne-tools#13228) Tidy up the directory reading (mne-tools#13268) FIX, DOC: Drop bad channel in 10_publication_figure.py (mne-tools#13266) ...
Reference issue
Fixes #13252.
What does this implement/fix?
Allows user to set
proj_id
andproj_name
through direct dict access.Ensures that
proj_id
is either an integer or None.Ensures that
proj_name
is either a string or None.Additional information
Example: