Add `raw.rescale` #13018

cbrnr · 2024-12-10T10:26:58Z

cbrnr · 2024-12-10T11:06:22Z

On a related note, could someone who uses Visual Studio Code try to debug my newly added test? Somehow, the debugger seems to be stuck when I (1) set a breakpoint in line 1070 and (2) then debug the test. Running the test does work though! Debugging tests work in other projects, so I was assuming that it's something specific to MNE. And yes, I already removed the venv and created a new one, basically using uv venv; source .venv/bin/activate; uv pip install -e ".[test]"; uv pip install pyside6. I also tried Python 3.13 and 3.12, and I also removed our custom pytest setup.

Screen.Recording.2024-12-10.at.12.03.09.mov

drammock · 2024-12-10T16:26:50Z

could someone who uses Visual Studio Code try to debug my newly added test?

same result as you. I seem to be hitting microsoft/debugpy#1623

./mne/io/tests/test_raw.py::test_rescale Failed: [undefined]matplotlib._api.deprecation.MatplotlibDeprecationWarning: The interactive_bk attribute was deprecated in Matplotlib 3.9 and will be removed in 3.11. Use ``matplotlib.backends.backend_registry.list_builtin(matplotlib.backends.BackendFilter.INTERACTIVE)`` instead.
mne/io/tests/test_raw.py:1070: in test_rescale
    raw = read_raw_fif(raw_fname, preload=True)  # multiple channel types
<stringsource>:69: in cfunc.to_py.__Pyx_CFunc_b0409f__29_pydevd_sys_monitoring_cython_object__lParen__etc_to_py_4code_4line.wrap
    ???
_pydevd_sys_monitoring\\_pydevd_sys_monitoring_cython.pyx:1429: in _pydevd_sys_monitoring_cython._line_event
    ???
_pydevd_sys_monitoring\\_pydevd_sys_monitoring_cython.pyx:1471: in _pydevd_sys_monitoring_cython._internal_line_event
    ???
_pydevd_sys_monitoring\\_pydevd_sys_monitoring_cython.pyx:1278: in _pydevd_sys_monitoring_cython._stop_on_breakpoint
    ???
_pydevd_sys_monitoring\\_pydevd_sys_monitoring_cython.pyx:1906: in _pydevd_sys_monitoring_cython._do_wait_suspend
    ???
/home/drmccloy/.vscode/extensions/ms-python.debugpy-2024.12.0-linux-x64/bundled/libs/debugpy/_vendored/pydevd/pydevd.py:2197: in do_wait_suspend
    keep_suspended = self._do_wait_suspend(thread, frame, event, arg, trace_suspend_type, from_this_thread, frames_tracker)
/home/drmccloy/.vscode/extensions/ms-python.debugpy-2024.12.0-linux-x64/bundled/libs/debugpy/_vendored/pydevd/pydevd.py:2223: in _do_wait_suspend
    self._activate_gui_if_needed()
/home/drmccloy/.vscode/extensions/ms-python.debugpy-2024.12.0-linux-x64/bundled/libs/debugpy/_vendored/pydevd/pydevd.py:1685: in _activate_gui_if_needed
    activate_function()
/home/drmccloy/.vscode/extensions/ms-python.debugpy-2024.12.0-linux-x64/bundled/libs/debugpy/_vendored/pydevd/pydevd.py:1668: in <lambda>
    "matplotlib": lambda: activate_matplotlib(do_enable_gui),
/home/drmccloy/.vscode/extensions/ms-python.debugpy-2024.12.0-linux-x64/bundled/libs/debugpy/_vendored/pydevd/pydev_ipython/matplotlibtools.py:99: in activate_matplotlib
    is_interactive = is_interactive_backend(backend)
/home/drmccloy/.vscode/extensions/ms-python.debugpy-2024.12.0-linux-x64/bundled/libs/debugpy/_vendored/pydevd/pydev_ipython/matplotlibtools.py:59: in is_interactive_backend
    from matplotlib.rcsetup import interactive_bk, non_interactive_bk  # @UnresolvedImport
/opt/mambaforge/envs/mnedev/lib/python3.12/site-packages/matplotlib/_api/__init__.py:216: in __getattr__
    return props[name].__get__(instance)
/opt/mambaforge/envs/mnedev/lib/python3.12/site-packages/matplotlib/_api/deprecation.py:161: in __get__
    emit_warning()
/opt/mambaforge/envs/mnedev/lib/python3.12/site-packages/matplotlib/_api/deprecation.py:196: in emit_warning
    warn_deprecated(
/opt/mambaforge/envs/mnedev/lib/python3.12/site-packages/matplotlib/_api/deprecation.py:99: in warn_deprecated
    warn_external(warning, category=MatplotlibDeprecationWarning)
/opt/mambaforge/envs/mnedev/lib/python3.12/site-packages/matplotlib/_api/__init__.py:381: in warn_external
    warnings.warn(message, category, stacklevel)
E   matplotlib._api.deprecation.MatplotlibDeprecationWarning: The interactive_bk attribute was deprecated in Matplotlib 3.9 and will be removed in 3.11. Use ``matplotlib.backends.backend_registry.list_builtin(matplotlib.backends.BackendFilter.INTERACTIVE)`` instead.

cbrnr · 2024-12-10T16:55:55Z

Thanks @drammock. So what do we do? The issue is half a year old and it doesn't look like it's on the top of their to do list

drammock · 2024-12-10T17:08:26Z

Thanks @drammock. So what do we do? The issue is half a year old and it doesn't look like it's on the top of their to do list

IDK. I forked and patched debugpy locally, but haven't yet managed to get the debugpy extension to use the local, patched copy.

In the meantime, if instead of a breakpoint you add a 1/0 or raise RuntimeError or similar, then the "right-click -> debug test" works as expected (for me at least)

larsoner · 2024-12-10T17:11:39Z

Debugging tests work in other projects, so I was assuming that it's something specific to MNE.

It probably is due to our treating all uncaught warnings as errors. If you add a line to our exceptions here like:

    ignore:The interactive_bk attribute was deprecated.*:

Does it work? On at least get to another warning you could ignore?

mne/io/base.py

drammock · 2024-12-10T17:18:24Z

ignore:The interactive_bk attribute was deprecated.*:

I had tried adding to tool.pytest.ini_options (which didn't work) but maybe I was doing it wrong... adding

    # debugpy uses deprecated matplotlib API
    ignore:The interactive_bk attribute was deprecated.*:MatplotlibDeprecationWarning

to conftest does change things:

Error while running tests:
TypeError: Cannot read properties of null (reading 'testsuites')

not sure if that's progress or not 😕

larsoner · 2024-12-10T17:21:08Z

ignore:The interactive_bk attribute was deprecated.*:MatplotlibDeprecationWarning

I would naively expect this to be problematic because MatplotlibDeprecationWarning is not a fully qualified name or builtin, so the test suite shouldn't even run IIRC (might be what your error means, you might see it if you run pytest yourself?). You should need:

    ignore:The interactive_bk attribute was deprecated.*:matplotlib._api.deprecation.MatplotlibDeprecationWarning

Or similar.

drammock · 2024-12-10T17:38:02Z

I would naively expect this to be problematic

ah, of course.

in the end I had to capture another similar warning too:

    # debugpy uses deprecated matplotlib API
    ignore:The (non_)?interactive_bk attribute was deprecated.*:

now the debugger works correctly.

drammock · 2024-12-10T17:39:35Z

@cbrnr I just pushed the fix to this branch. Debugging should hopefully work correctly for you now.

mscheltienne · 2024-12-10T22:28:47Z

I've been hitting this issue on VSCode for a while, ignoring the error in the conftest.py is my go-to solution. I think it's present in the conftest.py of MNE-LSL for instance.

cbrnr · 2024-12-11T06:46:51Z

Thanks everyone for your help, I am really happy that I can debug tests again! I've included the warning as suggested and expanded the examples prose a bit, so this should be ready.

mne/io/base.py

drammock · 2024-12-11T19:58:32Z

mne/io/base.py

+            for ch_type, ch_scale in scale.items():
+                if ch_type not in self.get_channel_types():


this structure is error-prone for interactive computing. If someone had a Raw that contained EEG but not MEG, and for some reason did this:

raw.rescale({eeg=1e-6, mag=1e-15})

then the EEG channels would get modified in place, and then the call would error when it got to the mag entry. Thinking that it didn't work, the user might then do

raw.rescale({eeg=1e-6})

with the end result that the rescaling was done twice to the EEG channels, probably without the user realizing it.

Please refactor so that the ValueError gets raised before any channels get rescaled in-place.

Good catch, I'll change this tomorrow!

Co-authored-by: Daniel McCloy <[email protected]>

drammock

LGTM. My only hesitation is the param name scale, which is basically identical to what elsewhere we (I think?) consistently call scalings. To me scale is not obviously clearer (here or elsewhere) so its only advantage (I think?) is fewer characters. What do others think?

cbrnr · 2024-12-13T14:48:03Z

I do not really have a preference, but scalings is plural, which is a bit awkward when rescaling with a single factor (e.g. raw.rescale(scalings=1e-6)).

drammock · 2024-12-13T14:55:38Z

I do not really have a preference, but scalings is plural, which is a bit awkward when rescaling with a single factor (e.g. raw.rescale(scalings=1e-6)).

yeah that occurred to me, but the converse is also true (singular param name when passing a dict with multiple scaling values)

larsoner · 2024-12-13T15:12:49Z

I think scalings is probably more consistent with what we do elsewhere

cbrnr · 2024-12-13T15:32:55Z

I'll change it to scalings then.

cbrnr requested review from drammock, agramfort, mscheltienne, dengemann and larsoner as code owners December 10, 2024 10:26

larsoner reviewed Dec 10, 2024

View reviewed changes

mne/io/base.py Show resolved Hide resolved

larsoner modified the milestones: 1.10, 1.9 Dec 10, 2024

cbrnr and others added 7 commits December 11, 2024 07:47

Add raw.rescale

9510d45

Add changelog entry

7311760

Clearer docstring

02ee24d

ignore debugpy's bad MPL API call

8a9b578

FIX: Another

7250bc5

FIX: Fine

b19548e

Add warning and more explanation

a1f3695

cbrnr force-pushed the rescale branch from 6644469 to a1f3695 Compare December 11, 2024 06:47

drammock requested changes Dec 11, 2024

View reviewed changes

Update mne/io/base.py

83e963c

Co-authored-by: Daniel McCloy <[email protected]>

cbrnr and others added 3 commits December 11, 2024 22:01

Update mne/io/base.py

b591cad

Co-authored-by: Daniel McCloy <[email protected]>

Update mne/io/base.py

ea6fd3f

Co-authored-by: Daniel McCloy <[email protected]>

Raise ValueError earlier

59463b5

drammock approved these changes Dec 13, 2024

View reviewed changes

Rename scale to scalings

2759f79

drammock approved these changes Dec 13, 2024

View reviewed changes

drammock enabled auto-merge (squash) December 13, 2024 15:58

larsoner and others added 2 commits December 14, 2024 15:50

Merge branch 'main' into rescale

934af16

Merge branch 'main' into rescale

99ede2a

drammock merged commit 521d667 into mne-tools:main Dec 14, 2024
29 checks passed

cbrnr deleted the rescale branch December 15, 2024 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `raw.rescale` #13018

Add `raw.rescale` #13018

cbrnr commented Dec 10, 2024

cbrnr commented Dec 10, 2024

drammock commented Dec 10, 2024

cbrnr commented Dec 10, 2024

drammock commented Dec 10, 2024

larsoner commented Dec 10, 2024 •

edited

Loading

drammock commented Dec 10, 2024

larsoner commented Dec 10, 2024

drammock commented Dec 10, 2024 •

edited

Loading

drammock commented Dec 10, 2024

mscheltienne commented Dec 10, 2024

cbrnr commented Dec 11, 2024

drammock Dec 11, 2024

cbrnr Dec 11, 2024

cbrnr Dec 13, 2024

drammock left a comment

cbrnr commented Dec 13, 2024

drammock commented Dec 13, 2024

larsoner commented Dec 13, 2024

cbrnr commented Dec 13, 2024

		for ch_type, ch_scale in scale.items():
		if ch_type not in self.get_channel_types():

Add raw.rescale #13018

Add raw.rescale #13018

Conversation

cbrnr commented Dec 10, 2024

cbrnr commented Dec 10, 2024

drammock commented Dec 10, 2024

cbrnr commented Dec 10, 2024

drammock commented Dec 10, 2024

larsoner commented Dec 10, 2024 • edited Loading

drammock commented Dec 10, 2024

larsoner commented Dec 10, 2024

drammock commented Dec 10, 2024 • edited Loading

drammock commented Dec 10, 2024

mscheltienne commented Dec 10, 2024

cbrnr commented Dec 11, 2024

drammock Dec 11, 2024

Choose a reason for hiding this comment

cbrnr Dec 11, 2024

Choose a reason for hiding this comment

cbrnr Dec 13, 2024

Choose a reason for hiding this comment

drammock left a comment

Choose a reason for hiding this comment

cbrnr commented Dec 13, 2024

drammock commented Dec 13, 2024

larsoner commented Dec 13, 2024

cbrnr commented Dec 13, 2024

Add `raw.rescale` #13018

Add `raw.rescale` #13018

larsoner commented Dec 10, 2024 •

edited

Loading

drammock commented Dec 10, 2024 •

edited

Loading