Add sourcespace to Report#12848
Conversation
for more information, see https://pre-commit.ci
|
For Another place it would be nice to add if you're motivated is in Then it would be good to pass some |
|
Having looked at the many possibilities, I think the best solution would be to add a figure in the This solution has the advantage of being able to also display the EEG sensors projected onto the scalp, as well as displaying only the sources used in for forward computation (as some sources from the original source space may be dropped if too close to the inner skull surface). This will give a global idea of the sensors <-> sources model. mne.viz.plot_alignment(trans=fwd["mri_head_t"], info=fwd["info"], src=fwd["src"], eeg=dict(original=0.2, projected=0.8))
We could also have a clearer view of the source space with an additional plot:
|
|
I like both of those plots! Adding to the |
This reverts commit 7c80b70.
for more information, see https://pre-commit.ci
…into dev-report-src
…into dev-report-src
for more information, see https://pre-commit.ci
…into dev-report-src
|
vferat
requested review from
agramfort,
drammock and
larsoner
as code owners
|
@vferat looks like some tests broke, can you check? |
|
@vferat it would be good to get this into 1.10 which we plan to release soon, do you have time to look at the failed tests? If not, can I take a look and see if I can fix them and push some commits? |
|
I will try to have a look during the weekend and let you now on Monday if I can't find time to finish this |
for more information, see https://pre-commit.ci
…into dev-report-src
for more information, see https://pre-commit.ci
|
Might need some help for the failed ubuntu jobs since I don't have a Linux distro with desktop environment available |
| replace=replace, | ||
| ) | ||
|
|
||
| if subject: |
There was a problem hiding this comment.
Hmm on second thought I think this is causing failures in CircleCI
Not sure we should make add_forward behavior dependent on whether or not subject is passed. Technically it could be pulled from fwd["src"][0]["subject_his_id"] in most cases. I'll think on it a bit and maybe make this part opt-in with plot=False (default) | True for report.add_forward, especially since report.add_bem already exists which gives some overlapping behavior.
| self._add_bem( | ||
| subject=subject, | ||
| subjects_dir=subjects_dir, | ||
| src=src, | ||
| trans=trans, | ||
| decim=1, | ||
| n_jobs=1, | ||
| width=512, | ||
| image_format=image_format, | ||
| title="Source space(s) (BEM view)", | ||
| section=section, | ||
| tags=tags, | ||
| replace=replace, | ||
| ) |
There was a problem hiding this comment.
Okay it's this step that takes over 10 minutes (!) to compute on CIs. Since we already have report.add_bem, I don't think we actually want this here.
Rather than make this depend on subject, too, I'm going to make it not depend on subject, but add a separate plot=True by default. That way if people want the old behavior, they can get it with plot=False. And if subject is not defined, it can be pulled from the source space.
|
Hey @larsoner, sorry to have left you on your own on that one, it was hard to find some time to finish this. Thanks for the help |
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
3 participants
Reference issue (if any)
Fixes #12836
What does this implement/fix?
Add a
srcargument to theReport.add_bemmethod.Additional information
I'm not sure which strategy to adopt and changes to make to
Report.parse_folder.The
Report.parse_foldermethod automatically adds BEM to theReportifReport.subjectis specified. However, several sourcespaces-src.fiffiles might be present in the BEM folder and/or folder to parse.