Skip to content

Conversation

@mscheltienne
Copy link
Member

Fixes this deprecation across the code base, observed in numpy 2.5+ (pre-release) version.

 .venv/lib/python3.13/site-packages/mne/_fiff/tag.py:180: in _read_matrix
    data.shape = dims
    ^^^^^^^^^^
E   DeprecationWarning: Setting the shape on a NumPy array has been deprecated in NumPy 2.5.
E   As an alternative, you can create a new view using np.reshape (with copy=False if needed).

CI workflow showing the failure: https://github.com/mne-tools/mne-lsl/actions/runs/20914667465/job/60085466508?pr=497

@mscheltienne
Copy link
Member Author

@larsoner
Copy link
Member

These should all have copy=False, at the very least in any code I wrote. That's the backward compatible behavior. I often made intentional use of the fact that setting that shape attribute was guaranteed to create a view rather than possibly copy.

@mscheltienne
Copy link
Member Author

mscheltienne commented Jan 12, 2026

I was wondering if I should have the copy=False, it is backward compatible as it would raise if it can't reshape without a copy, but wouldn't it be better to (1) try to get a view and (2) if we can't, copy and continue (which I think is what .reshape(...) now does under the hood)? I feel like this is an enhancement on the previous behavior.

@mscheltienne
Copy link
Member Author

Docstring:

copy : bool, optional
    If True, then the array data is copied. 
    If None, a copy will only be made if it’s required by order. 
    For False it raises a ValueError if a copy cannot be avoided. 
    Default: None.

@larsoner
Copy link
Member

np.reshape always tried to create a view and if it couldn't it would copy. In code I wrote setting .shape was just a shortcut to copy=False behavior and was done explicitly to say: "this should always be a view". If it's ever not, it's good that an error is raised because we're being unnecessarily inefficient and possibly buggy -- in many cases where the view-ness is utilized by inplace modifications of data. So by not setting view=False now, you're less explicit about what should always be guaranteed to happen (and if it doesn't we should make it so) and worse if a view is not created (which this PR will now allow silently!) then some computations will actually be incorrect.

@mscheltienne
Copy link
Member Author

Alright, I'll switch to explicitly raising if we can't get a view.

@mscheltienne
Copy link
Member Author

@larsoner Should be good this time- with also PRs in vtk and nitime.

@mscheltienne
Copy link
Member Author

Soo.. copy is 2.1+. I added a compatibility function to retain the old behavior across numpy versions.

@larsoner
Copy link
Member

Yaaay 🫠

Thanks for taking care of this @mscheltienne !

@larsoner larsoner merged commit f133bea into mne-tools:main Jan 12, 2026
32 checks passed
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.

2 participants