-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Update scalar representations as per NEP 51 #22449
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
342b701
to
ff552fc
Compare
@seberg so far so good for latest
|
So, I realized that the empty format string I don't really have an opinion, could even say that EDIT: Reconsidering this, maybe The thing to look into now (besides test fixes), is that array float formatting doesn't match scalar float formatting, so I actually need to pass through the formatting information better (should not be hard, but needs a bit organizing/thought.) |
893d77d
to
387e26a
Compare
This is starting to settle, although I am considering changing @rossbar do you know why the refguide check does not seem to notice changes from |
ce4166f
to
9a64758
Compare
I saw #22261 only now, so I won't complain if this is too late, but I'd really like to get away from |
I am proposing to print the bit-sized version, even if imprecise sometimes. Yes, it would be nice to change it, but it seems like a big ABI change on the C-side? |
Looked more closely, looks fine.
I'm afraid so, but I still think it'd be worth it eventually (numpy 2.0 material?). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a mismatch between the implementation and the NEP w.r.t. whether printing is affected or not.
Based on the implementation, I think this PR has pretty high chance of disrupting services where something gets calculated with numpy, and then passed on in some serialized form to another consumer, where numpy (or even python) might not necessarily be installed.
Is there any fallback we could provide so that people can adapt such interfaces (e.g. .value
or .raw
or ...)?
It won't, as long as |
Needs rebase. |
decdee0
to
08c07ce
Compare
08c07ce
to
bf92d6e
Compare
@mhvk , how do you think |
Looks like we're headed to green with |
I will note that just setting the legacy mode for the tests will unfortunately hide issues if you use |
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail. The doctest for tensor.extra_ops.Unique was failing because the output shape for the inverse indices has changed when axis is None: numpy/numpy#20638
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
From numpy PR numpy/numpy#22449, the repr of scalar values has changed, e.g. from "1" to "np.int64(1)", which caused two doctests to fail.
This is a WIP for implementing NEP 51/gh-22261. There are a few things that go beyond just changing the scalar representation:
It makes
get_formatter()
a more central/public API, to also movefmt=...
to it.dtype
is known, we can e.g. represent a longdouble as'3.1'
(with quotes) rather thannp.longdouble('3.1')
.fmt
is mainly used forfmt="r"
(previous point), I thinkfmt="s"
should be supported, but always be identical tostr(arr[0])
.fmt
path to be a ufunc (it should not inspect array values, unlike the default printing).fmt
should be used more cleanly for the__format__
work. This should be fine, I think it only means pushing the "option" adjustments lower.arr.tofile()
usedrepr(arr.item(...))
to print each element. This is weird/problematic:np.longdouble('3.1')
.'string'
orb'string'
Record and MA printing has to be adjusted to use the new functionality. This affects record scalars and the printing of the masked array
fill_value
.After some back and forth, I think the default should just be to go back to
str()
(this was changed silently in BUG 4381 Longdouble from string without precision loss #6199).str(arr.item())
is not ideal forfloat32
for example.This could use some thought (or should change to use the above
get_formatter(fmt="s")
, but I hope this can be decoupled from NEP 51.I plan to update the NEP 51 draft to include these points. Happy to discuss here of course (will make pushing NEP 51 quicker). But for now the draft-PR exists primarily to help with the NEP 51 discussion.
TODOS::
legacy
print-mode (i.e. to allow inclusion in the next release, but only switch with NEP 50 – which would likely be a NumPy 2.0 as well)arr.tofile()
is not well tested (which is why the change here should not fail tests).