Fix pick channels crash when _orig_units is None (#11314) - #14006
Conversation
| import numpy as np | ||
|
|
||
| import mne |
There was a problem hiding this comment.
No need to nest these they're required by MNE
c39e6c3 to
1e8c790
Compare
| info = mne.create_info(["Fp1", "Fp2", "F3", "F4"], 100.0, "eeg") | ||
| raw = mne.io.RawArray(np.zeros((4, 100)), info) |
There was a problem hiding this comment.
create_info and RawArray are already imported directly at top of file
| info = mne.create_info(["Fp1", "Fp2", "F3", "F4"], 100.0, "eeg") | |
| raw = mne.io.RawArray(np.zeros((4, 100)), info) | |
| info = create_info(["Fp1", "Fp2", "F3", "F4"], 100.0, "eeg") | |
| raw = RawArray(np.zeros((4, 100)), info) |
| from numpy.testing import assert_allclose, assert_array_equal, assert_equal | ||
| from scipy.io import savemat | ||
|
|
||
| import mne |
There was a problem hiding this comment.
unnecessary; the two API items you use (RawArray and create_info) are already being imported directly, just a few lines down.
Address @drammock review on mne-tools#14006: create_info and RawArray are already imported directly at the top of the test module, so call them directly and remove the now-unused `import mne`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyuFNWN45FNffpGwsC4Su7
23ae216 to
faddba6
Compare
Address @drammock review on mne-tools#14006: create_info and RawArray are already imported directly at the top of the test module, so call them directly and remove the now-unused `import mne`.
Picking/reordering channels built {k: v for ... in self._orig_units.items()}
unconditionally, raising AttributeError when _orig_units is None (as produced
by some readers / RawArray flows). Guard for a falsy _orig_units. Adds a test.
Address @drammock review on mne-tools#14006: create_info and RawArray are already imported directly at the top of the test module, so call them directly and remove the now-unused `import mne`.
faddba6 to
2764419
Compare
Address @drammock review on mne-tools#14006: create_info and RawArray are already imported directly at the top of the test module, so call them directly and remove the now-unused `import mne`.
2764419 to
c3a789b
Compare
|
@drammock — the nested I can't re-request review via the API as a non-collaborator, so flagging it here instead — this has been sitting in changes requested since 10 July with nothing actually outstanding. Sorry for not surfacing it sooner. AI-assisted, human-reviewed. |
|
Okay I think the review comments have been addressed so this can go in, thanks @CedricConday ! |
Fixes #11314
What was wrong
When picking/reordering channels on a
Raw, the code rebuilds_orig_units:If
self._orig_unitsisNone(which happens with some readers andRawArrayflows), this raisesAttributeError: 'NoneType' object has no attribute 'items'. The pick itself otherwise succeeds.Fix
Only rebuild
_orig_unitswhen it is truthy; leaveNone/empty as-is.Test
Adds
test_pick_channels_orig_units_none: picking with_orig_units = Noneno longer raises. Verified it raisesAttributeErroronmainand passes with this change.AI-assisted, human-reviewed — I'm an AI engineer; I find, fix, and test with AI (Claude Code), then review and verify before opening.