Disable view safety checks #6562
Conversation
579303b to
6602a20
Compare
6602a20 to
393934b
Compare
Remove unit tests for the view safety chekcs, which are to be reverted in the next commit.
4538d25 to
7ad5285
Compare
|
OK, I think this is good. Somewhat suprisingly the only issue that needs to be reopened is #2346, and I suspect even that will be fixed by #6053 without needing safety checks. So it looks like the safety checks weren't actually needed to solve any open issues - we just needed smarter logic to handle the case of object arrays in various places. Hmm... |
|
|
||
| if oldtype.names: | ||
| for name in oldtype.names: | ||
| if (oldtype.fields[name][1] == offset and |
There was a problem hiding this comment.
This looks a little funny. Why is it safe to return on first instance that the condition is satisfied?
There was a problem hiding this comment.
Just finding somthing that matches new type?
There was a problem hiding this comment.
The idea is to allow getfield as long as we are getting a field that already exists. Eg,
>>> ones(3, dtype='i4,O').setfield(5, 'O', 4)
Since get/setfield doesn't know the name of the field we have to search for the field based on the offset.
|
I like that so much code is removed ;) |
Because of slowdowns caused by the view safety checks introduced in numpy#5548 they are removed here for 1.10. The plan is to reintroduce a better version of the checks in 1.11.
7ad5285 to
796a5f8
Compare
|
removed the blank line |
|
@ahaldane Thanks for getting this done. And the removal of code always gives me a warm, fuzzy feeling ;) |
|
Thanks @charris, and agreed! Ultimately I think we will still want to allow view of arrays with objects at some point, but I'm hoping that will be easier under a new dtype system (in a year or two?), without needing these convoluted safety checks. |
|
Thanks @ahaldane ! I'm sorry this ended up being such a hassle for you. |
This PR disables the view safety checks that were causing problems in #6467, undoing some of the work from #5548. It goes back to the old behavior of disallowing any views/setfield involving object arrays.
This is better than reverting #5548, because it turns out there were other fixes in #5548 and subsequent PRs (eg #6208), plus in this PR, which fix most of the issues involving view safety without needing the safety checks. In fact, of the 5 issues fixed in #5548, only one needs to be re-opened due to this PR (#2599). (Edit: Actually #2346 needs to be re-opened too. Half of the examples there work now, but some are broken.)
I also request a day or two to sit on this before merging, please! (Ping me if I am delaying the release)