-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: prevent ufunc output to an already-broadcasted array #11667
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
Conversation
f015acb
to
f0be87f
Compare
f0be87f
to
159c72e
Compare
It turns out not so trivial to find the equivalent of "writable operand with 0 stride and usable dim > 0."
|
Some of that we can handle, but my guess is its either more complicated to detect (or needs a flag), or just doesn't belong in nditer but instead in the ufunc machinery in this case. Not sure though. Other pain points could be e.g. EDIT: Ah well, dunno, have to think a bit more to recall how this all worked.... |
This may indeed be better done in the ufunc setup - the fact that reduce uses this suggests that for the iterator one should continue to allow maximum freedom. Also, a different and I think better solution is to just make broadcasted arrays read-only; this is indeed why |
6b1d3a1
to
52308ca
Compare
rebased to fix merge conflicts |
52308ca
to
c1104ee
Compare
c1104ee
to
4249629
Compare
I moved the check to the ufunc level, added the trivial test from the issue and modified |
I think I may have to think about it a while when I feel I have the time, but maybe Marten has a feel for it. First, I want to think a bit whether this approach is right (and if you were maybe right after all with the nditer approach). Second, I am a bit unsure whether a hard error is good. |
I'm still not sure about this - I think I'd prefer to just not generate any writable broadcast arrays by default, and leaving any other usage up to the user, i.e., rather than prevent doing things that are potentially risky and fragile, make it such that it is a conscious choice to do so. |
Closing. If any fix is needed it should be done in |
Fixes #2705.
I tried to find a balance between fixing all possible edge cases and keeping the code simple. The documentation for broadcast_arrays already warns against writing to these, this PR attempts to detect a pending, perhaps unintentional write in a func operation and raises a
ValueError