BUG: Fix fill violating read-only flag. #22959
Conversation
- See numpy#22922 - `PyArray_FillWithScalar` checks if destination is writeable before attempting to fill it - A relevant test is added as a method of `TestRegression`
|
@seberg I guess this does the job, although I am not sure if checking whether the destination is writable should be done inside Also, wonder if |
numpy/core/src/multiarray/convert.c
Outdated
|
|
||
| if (PyArray_FailUnlessWriteable(arr, "assignment destination") < 0) { | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
The comment refers to the allocation below, so either put this check above the comment, or move the check after the first block of variable declarations.
numpy/core/tests/test_regression.py
Outdated
| # Ticket #22922 | ||
| with pytest.raises(ValueError): | ||
| a = np.zeros(11) | ||
| a.setflags(write=False) |
There was a problem hiding this comment.
Please move this out of the context, best to make the test as specific as possible? Also we usually put gh-22922 (and maybe "issue" rather than ticket, but it doesn't matter). "Ticket #..." sounds like it predates github :).
I would slightly prefer to move the test into test_multiarray.py which has a few fill related tests.
-- The comment in `PyArray_FillWithScalar` is about the code after the readonly check, so it was moved to the appropriate location. -- The `test_fill_readonly` was moved to test_multiarray.TestAttributes.
|
@seberg I modified the code as requested (I believe). |
|
Thanks @panosz those were the changes I was looking for exactly. |
seberg
changed the title
PyArray_FillWithScalar checks if destination is writeable before attempting to fill it. A relevant test is added as a method of TestRegression Closes numpygh-22922 Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
BUG: Fix fill violating read-only flag. (#22959)
PyArray_FillWithScalarchecks if destination is writeable before attempting to fill itTestRegression