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

Fix MNE example to work with MNE 1.3 #259

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

charlesbmi
Copy link
Contributor

@charlesbmi charlesbmi commented Apr 6, 2023

MNE 1.3 has fully deprecated psd_welch. Following the MNE 1.2 deprecation suggestion: https://mne.tools/1.2/generated/mne.time_frequency.psd_welch.html , I replaced it with .compute_psd(method="welch")

Additionally, paths are now returned using pathlib.Path, not str.

I ran this locally to check, and it works with MNE 1.3.1

@TomDonoghue TomDonoghue added the 1.1 Targetted for a fooof 1.1 release. label Jun 28, 2023
@fooof-tools fooof-tools deleted a comment from codecov-commenter Jun 29, 2023
@TomDonoghue
Copy link
Member

Hey @charlesincharge - thank you for catching & fixing this!

Sorry for a bit of a delay in getting back to you in this - we are currently working on a push to merge some updates, so I checked through this and made some direct edits / additions.

For tracking, what I updated is:

  • add a note that this example requires mne >= 1.2 (as this is when the current psd estimation function was added)
  • updated / cleaned up some imports and other mne stuff that is a bit outdated (picks), and updated to use Path objects (as you noted)

I'm going to merge this in now, but if you have any thoughts / comments on the updates, feel free to add some comments!

Thank you for your contribution to the fooof package!

@TomDonoghue TomDonoghue merged commit 117cbc0 into fooof-tools:main Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.1 Targetted for a fooof 1.1 release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants