NEP 50 and integers (e.g. uint8 together -1)
Hi all, In NEP 50 (https://numpy.org/neps/nep-0050-scalar-promotion.html) my current proposal is that the following: np.array([1, 2], dtype="uint8") + (-1) which currently returns an "int16" array must raise an error. This is because we want to return a uint8 array but the -1 cannot be represented well by -1. The same should also happen for a value of 300 (rather than -1). My main question is not about making this an error though. The question is whether the following two cases should also error: uint8_arr = np.array([1, 2], dtype="uint8") uint8_arr[0] = -1 or: np.array([-1], dtype=np.uint8) (In practice these currently give the maximum integer with well defined integer overflow.) Note that the call `np.uint8(-1)` could be a special case here! The reason I ask is that my PR: https://github.com/numpy/numpy/pull/21875 currently introduces the new error without changing the other cases and this requires adding a full new conversion path. If we want to change that anyway (or keep things aligned), I could simplify the logic in the PR. Cheers, Sebastian
Hi Sebastian, On Wed, Sep 28, 2022, at 12:11, Sebastian Berg wrote:
np.array([1, 2], dtype="uint8") + (-1)
which currently returns an "int16" array must raise an error. This is because we want to return a uint8 array but the -1 cannot be represented well by -1.
Did you mean: the -1 is not representable in uint8? Since -1 cannot cast to uint8, and since we cannot look at the value, we cannot determine a suitable minimal output dtype for x - 1. Stéfan
On Wed, 2022-09-28 at 16:44 -0700, Stefan van der Walt wrote:
Hi Sebastian,
On Wed, Sep 28, 2022, at 12:11, Sebastian Berg wrote:
np.array([1, 2], dtype="uint8") + (-1)
which currently returns an "int16" array must raise an error. This is because we want to return a uint8 array but the -1 cannot be represented well by -1.
Did you mean: the -1 is not representable in uint8?
Sorry yes. With NEP 50, we do not look at the value (initially) so determine that the operation must be handled as: uint8 + uint8 -> uint8 We then try to convert the -1 to uint8. That conversion would raise an error, because previously the result was an int16. (This is to prevent silent unexpected result changes and seemed like the more reasonable behavior.) On the other hand, assignments like: uint8_arr[0] = -1 np.array([-1], dtype=np.uint8) do happily convert the -1 to `uint8`, currently. If we keep allowing these, we have two slightly different conversions: one that fails and one that does not. This is fine, but maybe we actually want it to always fail in the future? - Sebastian
Since -1 cannot cast to uint8, and since we cannot look at the value, we cannot determine a suitable minimal output dtype for x - 1.
Stéfan _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: sebastian@sipsolutions.net
On Thu, Sep 29, 2022 at 10:10 AM Sebastian Berg <sebastian@sipsolutions.net> wrote:
On Wed, 2022-09-28 at 16:44 -0700, Stefan van der Walt wrote:
Hi Sebastian,
On Wed, Sep 28, 2022, at 12:11, Sebastian Berg wrote:
np.array([1, 2], dtype="uint8") + (-1)
which currently returns an "int16" array must raise an error. This is because we want to return a uint8 array but the -1 cannot be represented well by -1.
Did you mean: the -1 is not representable in uint8?
Sorry yes. With NEP 50, we do not look at the value (initially) so determine that the operation must be handled as:
uint8 + uint8 -> uint8
We then try to convert the -1 to uint8. That conversion would raise an error, because previously the result was an int16. (This is to prevent silent unexpected result changes and seemed like the more reasonable behavior.)
On the other hand, assignments like:
uint8_arr[0] = -1 np.array([-1], dtype=np.uint8)
do happily convert the -1 to `uint8`, currently. If we keep allowing these, we have two slightly different conversions: one that fails and one that does not.
This is fine, but maybe we actually want it to always fail in the future?
It seems better to always raise an exception indeed. So if it simplifies your PR to do that now, I'd say go for it. Cheers, Ralf
Hi all, just to mention it, there is a PR ready on this: https://github.com/numpy/numpy/pull/22385 it necessecitates some changes to the NumPy test suite and will do similar at least for SciPy downstream I currently suspect we will give it a shot soon, but am not opposed at all with discussing especially allowing: np.uint8(-1) i.e. using the negative range of the corresponding signed integer explicitly for the `np.uint8`, `np.uint16`. But still disallow it for other operations like `np.array([-1], dtype=np.uint8)`. As I said, my main interest is pushing NEP 50 related change: https://github.com/numpy/numpy/pull/21875 which this will simplify. If this PR seems tricky to move forward, that is fine, but I would like to push that PR without the simplification. (Decoupling this change from NEP 50 at some complexity cost.) Cheers, Sebastian On Tue, 2022-10-04 at 17:24 +0200, Ralf Gommers wrote:
On Thu, Sep 29, 2022 at 10:10 AM Sebastian Berg <sebastian@sipsolutions.net> wrote:
On Wed, 2022-09-28 at 16:44 -0700, Stefan van der Walt wrote:
Hi Sebastian,
On Wed, Sep 28, 2022, at 12:11, Sebastian Berg wrote:
np.array([1, 2], dtype="uint8") + (-1)
which currently returns an "int16" array must raise an error. This is because we want to return a uint8 array but the -1 cannot be represented well by -1.
Did you mean: the -1 is not representable in uint8?
Sorry yes. With NEP 50, we do not look at the value (initially) so determine that the operation must be handled as:
uint8 + uint8 -> uint8
We then try to convert the -1 to uint8. That conversion would raise an error, because previously the result was an int16. (This is to prevent silent unexpected result changes and seemed like the more reasonable behavior.)
On the other hand, assignments like:
uint8_arr[0] = -1 np.array([-1], dtype=np.uint8)
do happily convert the -1 to `uint8`, currently. If we keep allowing these, we have two slightly different conversions: one that fails and one that does not.
This is fine, but maybe we actually want it to always fail in the future?
It seems better to always raise an exception indeed. So if it simplifies your PR to do that now, I'd say go for it.
Cheers, Ralf _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: sebastian@sipsolutions.net
Since I am still a bit on the fence with the deprecation (see also below), to mostly make the following Python integer conversions an error (for now deprecation): np.uint8(-1) np.array(50000, dtype=np.int8) I have created a Poll about the change in case anyone wants to give feedback but formulate arguments in a mail: https://discuss.scientific-python.org/t/deprecation-of-assigning-converting-... Cheers, Sebastian PS: One tangential reason why I am bringing it up, is that I am struggling with the notion of "casting safety" for these Python integers. I will bring this up again also, but it is not directly related. On Tue, 2022-10-11 at 14:00 +0200, Sebastian Berg wrote:
Hi all,
just to mention it, there is a PR ready on this:
https://github.com/numpy/numpy/pull/22385
it necessecitates some changes to the NumPy test suite and will do similar at least for SciPy downstream I currently suspect we will give it a shot soon, but am not opposed at all with discussing especially allowing:
np.uint8(-1)
i.e. using the negative range of the corresponding signed integer explicitly for the `np.uint8`, `np.uint16`. But still disallow it for other operations like `np.array([-1], dtype=np.uint8)`.
As I said, my main interest is pushing NEP 50 related change:
https://github.com/numpy/numpy/pull/21875
which this will simplify. If this PR seems tricky to move forward, that is fine, but I would like to push that PR without the simplification. (Decoupling this change from NEP 50 at some complexity cost.)
Cheers,
Sebastian
On Tue, 2022-10-04 at 17:24 +0200, Ralf Gommers wrote:
On Thu, Sep 29, 2022 at 10:10 AM Sebastian Berg <sebastian@sipsolutions.net> wrote:
On Wed, 2022-09-28 at 16:44 -0700, Stefan van der Walt wrote:
Hi Sebastian,
On Wed, Sep 28, 2022, at 12:11, Sebastian Berg wrote:
np.array([1, 2], dtype="uint8") + (-1)
which currently returns an "int16" array must raise an error. This is because we want to return a uint8 array but the -1 cannot be represented well by -1.
Did you mean: the -1 is not representable in uint8?
Sorry yes. With NEP 50, we do not look at the value (initially) so determine that the operation must be handled as:
uint8 + uint8 -> uint8
We then try to convert the -1 to uint8. That conversion would raise an error, because previously the result was an int16. (This is to prevent silent unexpected result changes and seemed like the more reasonable behavior.)
On the other hand, assignments like:
uint8_arr[0] = -1 np.array([-1], dtype=np.uint8)
do happily convert the -1 to `uint8`, currently. If we keep allowing these, we have two slightly different conversions: one that fails and one that does not.
This is fine, but maybe we actually want it to always fail in the future?
It seems better to always raise an exception indeed. So if it simplifies your PR to do that now, I'd say go for it.
Cheers, Ralf _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: sebastian@sipsolutions.net
_______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: sebastian@sipsolutions.net
participants (3)
-
Ralf Gommers -
Sebastian Berg -
Stefan van der Walt