Skip to content

BUG,DEP: Fix writeable flag setting for arrays without base #13463

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

Merged
merged 1 commit into from
May 21, 2019

Conversation

seberg
Copy link
Member

@seberg seberg commented May 3, 2019

This also deprecates setting a non-writeable array to writeable if
that array does not own its data (and has no base object to check if
the memory may be writeable). (Deprecation on Python side only)

Closes gh-481


Well, this was probably a waste of time, thought it would be quicker, but should be reasonable in any case...

@seberg
Copy link
Member Author

seberg commented May 13, 2019

I may add a fix for Mattis PR here probably (with a test), but if someone reviews it before that, will just open a new PR afterwards.

1, &zero, NULL, &zero, flags, NULL);
}


Copy link
Member

Choose a reason for hiding this comment

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

Needs a comment what is the purpose of this function, it is a bit obscure at first glance.

"which do not own their data.") < 0) {
return NULL;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

The new deprecation should appear in the release notes

@mattip mattip added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label May 16, 2019
@seberg seberg force-pushed the writable-segfault branch 2 times, most recently from aa88116 to bba44ee Compare May 16, 2019 19:50
@seberg
Copy link
Member Author

seberg commented May 16, 2019

Add some docs and release notes (only changes).

--------------------------------------
When an array is created from the C-API to wrap already existing data and
the array does not own its data (frees it after destruction) it will not be
possible to switch its writeable flag to ``True`` in the future.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe clearer to state the reason:

"When an array is created from the C-API to wrap a pointer to data, the only indication we have of the read-write nature of the data is the writeable flag set during creation. It is dangerous to force the flag to writeable. In the future it will not be possible to switch the writeable flag to True.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that is better, added a "from python" at the end.

This also deprecates setting a non-writeable array to writeable if
that array does not own its data (and has no base object to check if
the memory may be writeable). (Deprecation on Python side only)

Closes numpygh-481
@seberg seberg force-pushed the writable-segfault branch from bba44ee to cc5b751 Compare May 16, 2019 20:25
@mattip mattip removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label May 21, 2019
@mattip mattip merged commit e4edcb7 into numpy:master May 21, 2019
@mattip
Copy link
Member

mattip commented May 21, 2019

Thanks @seberg, this also closes the second-oldest open issue!

@seberg seberg deleted the writable-segfault branch May 21, 2019 20:05
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label May 24, 2019
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.

segfault in _IsWriteable when data is not owned
3 participants