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

Add raw stc #12001

Merged
merged 17 commits into from
Oct 6, 2023
Merged

Add raw stc #12001

merged 17 commits into from
Oct 6, 2023

Conversation

BabaSanfour
Copy link
Contributor

Add functionality to extract raw time course of a brain region

In this commit, I have extended the script to include functionality that allows users to directly access the raw time courses of a specific brain region through its label without any transformation. This enhancement enables more flexible and detailed analysis of source data.

@BabaSanfour
Copy link
Contributor Author

Not sure if the 3.10 pip are failing from my part ?

@wmvanvliet
Copy link
Contributor

CI failure does not seem related to this PR.

I think it makes sense to add this functionality, although I was a bit confused by the term "raw" timecourses. I've changed the wording in the changes/devel.rst file when resolving a conflict there, but this is just a suggestion, feel free to change it again @BabaSanfour .

Could we add a unit test for the new functionality?

@BabaSanfour
Copy link
Contributor Author

@wmvanvliet the change should be fine ; The CI failure is fixed - somehow.

For the unit test I will add it later today. I found this comment "# XXX we don't check pca_flip, probably should someday...".
Should I add pca_flip unit test also ?

@wmvanvliet
Copy link
Contributor

If you could yes thank you :)

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

I assume you have some docstrings to fix. Can you check? eg regarding output shapes when using raw. thx

@BabaSanfour
Copy link
Contributor Author

Not sure the errors are related to my pull request; they in viz and transform tests which I didn't touch

@wmvanvliet
Copy link
Contributor

CI failures seem related:

 mne/tests/test_source_estimate.py:63:39: F401 [*] `mne.fixes._safe_svd` imported but unused
mne/tests/test_source_estimate.py:67:63: F401 [*] `mne.source_estimate._prepare_label_extraction` imported but unused

@BabaSanfour
Copy link
Contributor Author

Yes my bad,
I removed them

@drammock drammock enabled auto-merge (squash) October 6, 2023 12:03
Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

I changed "raw" to be None instead, after discussion with @larsoner, and then modified the logic accordingly. We agreed that None better reflected the user request, i.e., "no aggregation strategy across vertices".

marking for merge when CIs are green again, thanks @BabaSanfour!

@drammock drammock merged commit 37ae7e3 into mne-tools:main Oct 6, 2023
27 checks passed
@BabaSanfour
Copy link
Contributor Author

I agree it better reflects the logic of mne. Thanks for your work! Glad to contribute.

larsoner added a commit to larsoner/mne-python that referenced this pull request Oct 10, 2023
* upstream/main: (37 commits)
  Use constrained layout in matplotlib visualization (mne-tools#12050)
  Add raw stc (mne-tools#12001)
  [MRG] update codeowners (mne-tools#12089)
  DOC: Morlet wavelet length in tfr_morlet (mne-tools#12073)
  BUG: Fix bug with mne browser backend (mne-tools#12078)
  Cache avatars (mne-tools#12077)
  BUG: Fix bug with ch_name resolution (mne-tools#12086)
  add unicode roundtrip for FIF (mne-tools#12080)
  add Ivan to names.inc (mne-tools#12081)
  MAINT: Work around PySide 6.5.3 event loop error (mne-tools#12076)
  mne-tools#11608, buggfix and docstring update (mne-tools#12066)
  MAINT: Fix broken examples (mne-tools#12074)
  Add UI Event linking to DraggableColorbar (mne-tools#12057)
  handle lazy loading through .pyi type stubs (mne-tools#12072)
  BUG: Fix bug with sensor_colors (mne-tools#12068)
  clean  up some deprecations (mne-tools#12067)
  Allow not dropping bads when creating or plotting Spectrum objs (mne-tools#12006)
  Collapsible html repr for raw/info (mne-tools#12064)
  BUG: Fix bug with pickling MNEBadsList (mne-tools#12063)
  add details for Denis (mne-tools#12065)
  ...
@BabaSanfour BabaSanfour deleted the add_raw_stc branch October 12, 2023 13:38
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Marijn van Vliet <w.m.vanvliet@gmail.com>
Co-authored-by: Daniel McCloy <dan@mccloy.info>
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.

4 participants