Fix a bug for ica.get_sources() and add a corresponding test #13068
Conversation
BeiGeJin
requested review from
adam2392,
agramfort,
cbrnr,
dengemann and
mscheltienne
as code owners
|
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
larsoner
left a comment
larsoner
left a comment
There was a problem hiding this comment.
Looks like a good start, just a few minor comments!
You'll also need to add a doc/changes/devel/13068.bugfix.rst with a message like
Fixed ICA processing of concatenated raw instances, by :newcontrib:`Firstname Lastname`.then add your name to doc/changes/names.inc
for more information, see https://pre-commit.ci
Sure! I've got all these done according to your comments. |
* upstream/main: (230 commits) FIX: Fix ICA.apply when fitted including marked bad channels (mne-tools#13478) FIX: Correctly set the calibration factor in Nihon Kohden reader (mne-tools#13468) [pre-commit.ci] pre-commit autoupdate (mne-tools#13479) MAINT: Update code credit (mne-tools#13477) Fix `versionadded` directive formatting (mne-tools#13471) typo in mailmap (mne-tools#13475) FIX: Fix _plot_topomap channel names plotting when using a mask (mne-tools#13470) FIX: Handle an Eyelink File with blank lines injected throughout file (mne-tools#13469) ENH: adds annotation filtering to raw and ica source figures (mne-tools#13460) MAINT: Restore VTK nightly wheel on Linux and bump changelog checker (mne-tools#13436) [pre-commit.ci] pre-commit autoupdate (mne-tools#13465) FIX: Fix add_reference_channels for passing two channels names (mne-tools#13466) ENH: Add on_missing for combine_channels (mne-tools#13463) Bump the actions group with 2 updates (mne-tools#13464) Move development dependencies into a dependency group (no more extra) (mne-tools#13452) ENH: add on_missing for rename_channels (mne-tools#13456) add advisory board to website (mne-tools#13462) ENH: Support Nihon Kohden EEG-1200A V01.00 (mne-tools#13448) MAINT: Update dependency specifiers (mne-tools#13459) ENH: Add encoding parameter to Nihon Kohden reader (mne-tools#13458) ...
|
@BeiGeJin sorry we missed this one, we don't get notified when people push commits so didn't realize you had addressed our comments! I merged |
|
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
3 participants
Reference issue (if any)
Fixes #13057.
What does this implement/fix?
It fixes the bug in ica.get_sources() for concatenated raw object.
There is a bug in the ica._source_as_raw function, here it assigns [self.last_samp] to self._last_samps, however self.last_samp is a property function and it seems that if its wrapped in a list, it won't actually call this function until used later. So I change it to first calculate the values and then put them in the list.