Skip to content

DEP: Deprecate setting the shape attribute of an exact numpy array #29523

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

eendebakpt
Copy link
Contributor

Addresses part of #28800.

Notes:

  • for small arrays the changes could have a minor performance impact as x.reshape(shape) returns a view (e.g. a new object). We could optimize perhaps by returning the same array if the shape is unchanged, but I am not sure this is worth it.

  • Setting the shape in __array_finalize__ (e.g. here) is problematic: using reshape is not possible as we have to modify the self. This is seems rather safe (since we are still in __array_finalize__ it would be strange for other threads to already have access to the object), but we have to deal with it somehow.
    Some options: i) suppress the warning here ii) issue the warning only for exact np.ndarray iii) refactor the matrixlib (and others) to not set the shape iv) use in private method to set the shape.
    Option i) is not very performant iii) I do not know how to do, iv) might give issues with subclasses that override setting the shape (is that possible?). So I am picking option ii) for now (same approach as in the PR where dtype setting is deprecated

@eendebakpt eendebakpt changed the title Deprecate setting the shape attribute of a numpy array DEP] Deprecate setting the shape attribute of a numpy array Aug 6, 2025
@jorenham jorenham changed the title DEP] Deprecate setting the shape attribute of a numpy array DEP: Deprecate setting the shape attribute of a numpy array Aug 6, 2025
@ngoldbaum
Copy link
Member

Looks like there's one docstring that still needs an update:

_________________________ [doctest] basics.copies.rst __________________________
111 
112     >>> import numpy as np
113     >>> x = np.ones((2, 3))
114     >>> y = x.T  # makes the array non-contiguous
115     >>> y
116     array([[1., 1.],
117            [1., 1.],
118            [1., 1.]])
119     >>> z = y.view()
120     >>> z.shape = 6
Differences (unified diff with -expected +actual):
    @@ -1,4 +1,8 @@
     Traceback (most recent call last):
    -   ...
    -AttributeError: Incompatible shape for in-place modification. Use
    -`.reshape()` to make a copy with the desired shape.
    +  File "/home/circleci/.pyenv/versions/3.11.10/lib/python3.11/doctest.py", line 1355, in __run
    +    exec(compile(example.source, filename, "single",
    +  File "<doctest basics.copies.rst[22]>", line 1, in <module>
    +    z.shape = 6
    +    ^^^^^^^
    +DeprecationWarning: Setting the shape on a NumPy array has been deprecated in NumPy 2.4.
    +As an alternative, you can create a new view using np.reshape.

/home/circleci/repo/doc/source/user/basics.copies.rst:120: DocTestFailure

an array is unsafe if an array is shared, especially by multiple
threads. As an alternative, you can create a new view (no copy) via
`np.reshape` or `array.reshape`.

Copy link
Member

Choose a reason for hiding this comment

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

any chance you can add a more fleshed out concrete code example showing exactly what to do?

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if someone could write a ruff lint for this?

Copy link
Member

Choose a reason for hiding this comment

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

A NPY rule would indeed be nice. But those of us that use a type-checker should already see a deprecation message appear in their IDE.

Copy link
Member

Choose a reason for hiding this comment

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

The nice thing about adding ruff rules is it can automatically fix issues for you.

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a ruff rule though? You have a new object, so it just isn't a safe rule to just apply (in part for the exact same reason that we want this, because it is weird to change the shape on the other side of a program).

@seberg
Copy link
Member

seberg commented Aug 8, 2025

iv) might give issues with subclasses that override setting the shape (is that possible?)

It seems possible, but rather raw:

class arr(np.ndarray):
    @property
    def shape(self):
        return super().shape
    @shape.setter
    def shape(self, val):
        np.ndarray.shape.__set__(self, val)

(I suppose you may be able to walk the __mro__ like super there somehow). But it seems arcane enough that nobody should do it (and why would you?)).

I still think option iv might be nicer, just because the need for this should only arise in the subclass implementation itself and not for end-users. But happy to go with this initially (even this might be quite noisy, of course).

@eendebakpt
Copy link
Contributor Author

I still think option iv might be nicer, just because the need for this should only arise in the subclass implementation itself and not for end-users. But happy to go with this initially (even this might be quite noisy, of course).

For setting the shape the only complex part is in np.matrixlib which is already deprecated (7 years ago, maybe time to remove it?) so option iv) might indeed be better. I will open a PR with this approach.

@eendebakpt eendebakpt changed the title DEP: Deprecate setting the shape attribute of a numpy array DEP: Deprecate setting the shape attribute of an exact numpy array Aug 10, 2025
@seberg
Copy link
Member

seberg commented Aug 11, 2025

Matrix deprecation always waited on SciPy sparse matrix being replaced. That process finally started, but it may not be quite far enough yet to actually do it.
(Plus, maybe we need to escalate the deprecation a bit first considering the impact.)

@seberg
Copy link
Member

seberg commented Aug 11, 2025

Ah, sorry, I see you just had to change how the shape is handled there (i.e. that weird offender) and not actually need to get rid of it. To me that seems actually pretty nice and not too complex if it works!

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.

4 participants