Skip to content

Conversation

@SYXiao2002
Copy link
Contributor

Reference issue (if any)

Fixes #13117.

What does this implement/fix?

Enhanced the calculation of the sampling rate to avoid potential indexing errors.

Additional information

It is recommended to include rtol as a function parameter for _extract_sampling_rate(dat) in the future.

@SYXiao2002 SYXiao2002 requested a review from rob-luke as a code owner February 19, 2025 04:18
@welcome
Copy link

welcome bot commented Feb 19, 2025

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

Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

LGTM. I'm lukewarm on insisting on adding a test, since this was clearly just a suboptimal approach before. @larsoner WDYT?

Co-authored-by: Daniel McCloy <dan@mccloy.info>
@drammock
Copy link
Member

superseded by #13184

@drammock drammock closed this Apr 18, 2025
@larsoner larsoner reopened this Apr 18, 2025
@larsoner
Copy link
Member

I'll push a commit to update the changelog for #13184

* upstream/main: (40 commits)
  fix typo (missing space) that messed up rst rendering (mne-tools#13217)
  MAINT: Restore VTK dev (mne-tools#13214)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#13212)
  BUG: Fix bug with example (mne-tools#13210)
  MAINT: Fix pip-pre with PyVista (mne-tools#13207)
  Move FCBG to former partners (mne-tools#13205)
  ENH: Update related software list (mne-tools#13202)
  fix sfreq estimation for snirf files (mne-tools#13184)
  ENH: Use data-based padding instead of "odd" padding when filtering in raw.plot (mne-tools#13183)
  FIX: Bumps (mne-tools#13198)
  DOC: fix typo in examples/io/read_impedances.py (mne-tools#13197)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#13173)
  FIX make_watershed_bem to handle missing talairach_with_skull.lta courtesy Freesurfer 8 (mne-tools#13172)
  ENH: Add upsampling for MEG helmet surface (mne-tools#13179)
  MAINT: Update code credit (mne-tools#13180)
  BUG: Fix bug with least-squares sphere fit (mne-tools#13178)
  fix EDF export (mne-tools#13174)
  fix typo (mne-tools#13171)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#13164)
  Fix dev installation guide (mne-tools#13163)
  ...
@larsoner larsoner enabled auto-merge (squash) April 18, 2025 15:57
@larsoner larsoner disabled auto-merge April 18, 2025 15:57
@larsoner larsoner enabled auto-merge (squash) April 18, 2025 15:57
@larsoner larsoner disabled auto-merge April 21, 2025 16:33
@larsoner larsoner merged commit bd0da9b into mne-tools:main Apr 21, 2025
30 of 32 checks passed
@welcome
Copy link

welcome bot commented Apr 21, 2025

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

@larsoner
Copy link
Member

Thanks @SYXiao2002 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected Indexing Error Due to Low-Precision sampling_rate Calculation

3 participants