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 vmax slider for EvokedField figures #12354

Merged
merged 3 commits into from
Jan 16, 2024
Merged

Conversation

wmvanvliet
Copy link
Contributor

This fixes bugs introduced by shadowing the build-in type object. When adding unit tests, more bugs surfaced and were fixed.

fixes #12352

@wmvanvliet
Copy link
Contributor Author

I'm not sure we need a deprecation cycle for the type => kind change in the set_vmax function signature, since set_vmax was broken before anyway.

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 doc/changes/bugfix.rst to document the fix? (Or a doc/changes/12354.bugfix.rst if you don't want the bot to auto-rename it using your PR number?)

Otherwise LGTM

@wmvanvliet wmvanvliet requested a review from agramfort as a code owner January 12, 2024 19:06
@wmvanvliet
Copy link
Contributor Author

I don't think the CI failure is related to this PR?

@drammock
Copy link
Member

I don't think the CI failure is related to this PR?

those errors are gone from our code after #12353; but it looks like pyvista code is triggering them. I'll push a quick ignore-fix

@drammock
Copy link
Member

actually not so easy to ignore, as they're errors, not warnings. I've opened pyvista/pyvista#5472.

@drammock drammock enabled auto-merge (squash) January 15, 2024 18:12
@drammock drammock merged commit 6af181a into mne-tools:main Jan 16, 2024
25 checks passed
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
@wmvanvliet wmvanvliet deleted the fix-vmax branch July 18, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: flake in code due to shadowing of builtin
3 participants