ENH: Multiple raw instance support to head pos average #12445
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
larsoner
requested review from
agramfort,
alexrockhill,
dengemann and
drammock
as code owners
larsoner
changed the title
12 tasks
Member
Author
|
All green, ready from my end @drammock ! |
agramfort
approved these changes
Feb 16, 2024
Member
|
thx @larsoner ! |
drammock
reviewed
Feb 16, 2024
Comment on lines
+337
to
+338
| _validate_type(r, BaseRaw, f"raw[{ri}]") | ||
| _validate_type(p, np.ndarray, f"pos[{ri}]") |
Member
There was a problem hiding this comment.
the error messages here (which will reference raw[0] or pos[0]) might be slightly confusing for the case where a user passed only 1 raw / pos (not a list). But getting it "right" is pretty convoluted:
item_name = "raw" if len(raw) == 1 else "raw[{}]"
_validate_type(r, BaseRaw, item_name.format(ri))if you can think of a cleaner/simpler way please go for it, otherwise probably fine as-is.
| trans[:3, 3] = dt @ hp[:, 4:7] | ||
| dist = np.linalg.norm(trans[:3, 3]) | ||
| if dist > 1: # less than 1 meter is sane | ||
| warn(f"Implausible head position detected: {dist} meters from device origin") |
Member
There was a problem hiding this comment.
tempted to do this but it bumps the line over the length limit
Suggested change
| warn(f"Implausible head position detected: {dist} meters from device origin") | |
| warn(f"Implausible head position detected: {dist:0.3f} meters from device origin") |
Member
|
looks like this got merged while I was reviewing... feel free to ignore my comments (they were very minor / nitpicks anyway) |
Member
|
oups :)
… Message ID: ***@***.***>
|
Member
it's fine, no way you could have known! |
larsoner
added a commit
to SophieHerbst/mne-python
that referenced
this pull request
Feb 20, 2024
* upstream/main: [pre-commit.ci] pre-commit autoupdate (mne-tools#12453) MAINT: Fix CIs for PyQt6 (mne-tools#12452) DOC: add missing info to interpolate bads docstring (mne-tools#12448) FIX: na_rep for reports (mne-tools#12447) ENH: Multiple raw instance support to head pos average (mne-tools#12445) ENH: Add image_kwargs to report.add_epochs (mne-tools#12443)
snwnde
pushed a commit
to snwnde/mne-python
that referenced
this pull request
Mar 20, 2024
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needed for mne-tools/mne-bids-pipeline#574