Warn or immidiately change readonly flag on broadcast_arrays return value?
In PR 12609 https://github.com/numpy/numpy/pull/12609 I added code to emit a DepricationWarning when broadcast_arrays returns an array where the output is repeated. While this is a minimal fix to the problem, perhaps we should consider making the output readonly immediately instead? - A deprecation cycle requires two changes to downstream user's code: one to filter the deprecation warning, and another when we actually make the change - Writing to the repeated data will cause errors now. What do you think, should we change the behaviour at all, and if so should we depricate it over two releases or change it immediately? The original issue is here https://github.com/numpy/numpy/issues/2705 Matti
Hi! Broadcasting almost always returns a repeated output (except when all arrays are the same shape), that’s the entire point. I suspect this function is in fairly widespread use and will therefore cause a lot of downstream issues when repeating, so I’m -0.5 on a DeprecationWarning. A FutureWarning might be more appropriate, in which case I’m +0.2. As for making the output read-only, that might break code, but most likely the code was erroneous anyway. But breaking backward-compatibility without a grace period is unheard of in this community. I’m +0.5 on it anyway. 🤷🏻♂️ Overall, a kind of hairy problem with no clear solution. Best Regards, Hameer Abbasi
On Tuesday, Dec 25, 2018 at 12:13 PM, Matti Picus <matti.picus@gmail.com (mailto:matti.picus@gmail.com)> wrote: In PR 12609 https://github.com/numpy/numpy/pull/12609 I added code to emit a DepricationWarning when broadcast_arrays returns an array where the output is repeated. While this is a minimal fix to the problem, perhaps we should consider making the output readonly immediately instead?
- A deprecation cycle requires two changes to downstream user's code: one to filter the deprecation warning, and another when we actually make the change
- Writing to the repeated data will cause errors now.
What do you think, should we change the behaviour at all, and if so should we depricate it over two releases or change it immediately?
The original issue is here https://github.com/numpy/numpy/issues/2705
Matti
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
Probably this will cause a lot of groans, but I've definitely written code modifying `broadcast_to` outputs, intentionally. As such I am -1 on this whole endeavour. My preference on making arrays read-only is to have a very light touch if any. As an example, at some point `np.diag` started returning read-only views. Setting or modifying the diagonal of a matrix is a common operation, so this decision uglified my code (grab the diagonal, make it writeable, write to it, instead of `np.diag(M) = x`. I'll admit that the times that I modified the `broadcast_to` output I felt rather hacky and sheepish, but the point is that it's unlikely that someone who is using this function doesn't know what they're doing, isn't it? I wouldn't have even thought to look for this function before I understood what broadcasting and 0-strides were. In fact, I use it *specifically* to save memory and use zero strides. Matti, could you comment a bit on the motivation behind this change and why you feel it's necessary? Thanks, Juan.
On 25 Dec 2018, at 10:26 pm, Hameer Abbasi <einstein.edison@gmail.com> wrote:
Hi!
Broadcasting almost always returns a repeated output (except when all arrays are the same shape), that’s the entire point. I suspect this function is in fairly widespread use and will therefore cause a lot of downstream issues when repeating, so I’m -0.5 on a DeprecationWarning. A FutureWarning might be more appropriate, in which case I’m +0.2.
As for making the output read-only, that might break code, but most likely the code was erroneous anyway. But breaking backward-compatibility without a grace period is unheard of in this community. I’m +0.5 on it anyway. 🤷🏻♂️
Overall, a kind of hairy problem with no clear solution.
Best Regards, Hameer Abbasi
On Tuesday, Dec 25, 2018 at 12:13 PM, Matti Picus <matti.picus@gmail.com <mailto:matti.picus@gmail.com>> wrote: In PR 12609 https://github.com/numpy/numpy/pull/12609 I added code to emit a DepricationWarning when broadcast_arrays returns an array where the output is repeated. While this is a minimal fix to the problem, perhaps we should consider making the output readonly immediately instead?
- A deprecation cycle requires two changes to downstream user's code: one to filter the deprecation warning, and another when we actually make the change
- Writing to the repeated data will cause errors now.
What do you think, should we change the behaviour at all, and if so should we depricate it over two releases or change it immediately?
The original issue is here https://github.com/numpy/numpy/issues/2705
Matti
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
Hi Juan, I also use `broadcast_to` a lot, to save memory, but definitely have been in a situation where in another piece of code the array is assumed to be normal and writable (typically, that other piece was also written by me; so much for awareness...). Fortunately, `broadcast_to` already returns read-only views (it is newer and was designed as such from the start), so I was alerted. At the time `broadcast_to` was introduced [1], there was a discussion about `broadcast_arrays` as well, and it was decided to leave it writable - see the note at https://github.com/numpy/numpy/blob/master/numpy/lib/stride_tricks.py#L265. The same note suggests a deprecation cycle, but like Hameer I don't really see how that helps - there is no way to avoid the warning in the large majority of cases where readonly views are exactly what the user wanted in the first place. So, that argues for cold turkey... The one alternative would be to actually warn only if one attempts to write to a broadcast array - apparently this has been done for `np.diag` as well [2], so code to do so supposedly exists. All the best, Marten [1] https://github.com/numpy/numpy/pull/5371 [2] https://github.com/numpy/numpy/pull/5371#issuecomment-75238827
participants (4)
-
Hameer Abbasi -
Juan Nunez-Iglesias -
Marten van Kerkwijk -
Matti Picus