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.

Sign up for GitHub

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

Jump to bottom

BUG: Fix fill violating read-only flag. #22959

Merged
merged 4 commits into from Jan 8, 2023
Merged

BUG: Fix fill violating read-only flag. #22959

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
115bca2
Bug: Fix fill violating read-only flag.
panosz Jan 7, 2023
6cac473
MAINT: Fix comment in PyArray_FillWithScalar. Move test to new location.
panosz Jan 8, 2023
39200d8
Apply suggestions from code review
seberg Jan 8, 2023
a3136b5
Apply suggestions from code review
seberg Jan 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions numpy/core/src/multiarray/convert.c
View file
Open in desktop
Expand Up @@ -365,6 +365,10 @@ PyArray_FillWithScalar(PyArrayObject *arr, PyObject *obj)
*
* (The longlong here should help with alignment.)
*/

seberg marked this conversation as resolved.
Show resolved Hide resolved
if (PyArray_FailUnlessWriteable(arr, "assignment destination") < 0) {
return -1;
}
Copy link
Member

Choose a reason for hiding this comment

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

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.

npy_longlong value_buffer_stack[4] = {0};
char *value_buffer_heap = NULL;
char *value = (char *)value_buffer_stack;
Expand Down
7 changes: 7 additions & 0 deletions numpy/core/tests/test_regression.py
View file
Open in desktop
Expand Up @@ -323,6 +323,13 @@ def bfb():
assert_raises(ValueError, bfa)
assert_raises(ValueError, bfb)

def test_fill_readonly(self):
# Ticket #22922
with pytest.raises(ValueError):
a = np.zeros(11)
a.setflags(write=False)
Copy link
Member

Choose a reason for hiding this comment

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

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.

a.fill(0)

@pytest.mark.xfail(IS_WASM, reason="not sure why")
@pytest.mark.parametrize("index",
[np.ones(10, dtype=bool), np.arange(10)],
Expand Down