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

BUGFIX: return events if provided when current = desired sfreq #13070

Merged
merged 11 commits into from
Jan 24, 2025

Conversation

Randomidous
Copy link
Contributor

Reference issue (if any)

Fixes #13066

What does this implement/fix?

The resample function takes an additional parameter "events" and returns a resampled copy if provided. When the current is equal to the desired sfreq, only self is returned. This fix now returns raw and events if both were provided, avoiding preprocessing pipeline bugs down the line.

Additional information

A test for this is described in the issue.

in case current sfreq equals desired sfreq
Copy link

welcome bot commented Jan 17, 2025

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Can you add a tiny test that would fail on main but pass on this PR? Perhaps in mne/io/fiff/tests/test_raw_fiff.py:def test_resample?

doc/changes/devel/13070.bugfix.rst Outdated Show resolved Hide resolved
mne/io/base.py Outdated Show resolved Hide resolved
@Randomidous
Copy link
Contributor Author

Randomidous commented Jan 17, 2025

@larsoner Done! Test added under mne/io/fiff/tests/test_raw_fiff.py:def test_resample

@larsoner larsoner added the backport-candidate on-merge: backport to maint/1.9 label Jan 24, 2025
@larsoner larsoner merged commit 7daecee into mne-tools:main Jan 24, 2025
30 checks passed
Copy link

welcome bot commented Jan 24, 2025

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

@larsoner
Copy link
Member

Thanks @Randomidous !

meeseeksmachine pushed a commit to meeseeksmachine/mne-python that referenced this pull request Jan 24, 2025
larsoner pushed a commit that referenced this pull request Jan 24, 2025
…ided when current = desired sfreq) (#13081)

Co-authored-by: Roy Eric <[email protected]>
@Randomidous
Copy link
Contributor Author

My pleasure @larsoner :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-candidate on-merge: backport to maint/1.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants