10000 np.set_printoptions(sign='legacy') edge case for 0d array · Issue #9804 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

np.set_printoptions(sign='legacy') edge case for 0d array #9804

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.

Sign up for GitHub

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

Jump to bottom
Closed
lesteve opened this issue Oct 1, 2017 · 12 comments
Closed

np.set_printoptions(sign='legacy') edge case for 0d array #9804

lesteve opened this issue Oct 1, 2017 · 12 comments

Comments

@lesteve
Copy link
Contributor
lesteve commented Oct 1, 2017

After #9139 got merged, I tried using np.set_printoptions(sign='legacy') to see whether doctests were passing for scikit-learn with numpy > 1.13. I bumped into the following edge case:

numpy 1.13

import numpy as np
np.array(1e-10)

Output:

array(1e-10)

numpy 1.14.0.dev0

import numpy as np
np.set_printoptions(sign='legacy')
np.array(1e-10)

Output:

array(  1.00000000e-10)

I was expecting that with np.set_printoptions(sign='legacy'), the behaviour would match exactly on numpy 1.13 and numpy 1.14.0.dev0.

Note that it only happens if the number in the array is smaller than the suppress argument in np.set_printoptions. There are probably work-arounds we can use for scikit-learn doctests, like using np.set_printoptions(sign='legacy', suppress=True) to ensure the scientific notation is not used.

cc @ahaldane @eric-wieser.
@eric-wieser eric-wieser added this to the 1.14.0 release milestone Oct 1, 2017
@ahaldane
Copy link
Member
ahaldane commented Oct 2, 2017

For the 0d case you are talking about we are still working on two competing PRs, #9139 and #9201. (These were actually the motivation for all the printing changes). However, I think they both change the 0d printing relative to 1.13 close to the way you described.

In particular, in 1.13, a typical 0d float array prints as array(1.0), but with those PRs in 1.14 it will print array(1.). Maybe we can somehow finagle things so sign='legacy' changes them too, but it seems difficult, but we will think about it.

Essentially it boils down to the fact that the inner element of 0d and 1d arrays now prints the same, so we will get:

>>> np.array([1e-10])  # unchanged from 1.13 to 1.14 with sign=legacy
array([1.00000000e-10])
>>> np.array(1e-10)  # changes in 1.14 to match the 1d behavior
array(1.00000000e-10)

@ahaldane
Copy link
Member
ahaldane commented Oct 2, 2017

By the way, I was aware of this, which is why in the release note I said "setting sign='legacy' mostly reproduces the 1.13 behavior" :)

We'll see if we can remove the "mostly".

@eric-wieser
Copy link
Member

For the 0d case you are talking about we are still working on two competing PRs,

Surely those PRs only affect str, and this is about repr?

@ahaldane
Copy link
Member
ahaldane commented Oct 2, 2017

Oh that's true, I guess the current output was the plan for repr.

Still, maybe we can think of a way of tweaking sign=legacy to use the old .item() computation... I remember considering it but it didn't look super easy.

@eric-wieser
Copy link
Member

The leading space here looks like a clear bug, but I think the extra digits are a feature.

@ahaldane
Copy link
Member
ahaldane commented Oct 3, 2017

FWIW the output in my last comment (without the space) is the output of #9139, and therefore probably #9201 too. So I think the space is fixed.

@ahaldane
Copy link
Member
ahaldane commented Oct 3, 2017

Actually, probably an easy way to make the sign=legacy case use the old 0d code is just to put a special case in array2string like if legacy is True and arr.shape == (): return style(x.item()).

In that case instead of using sign='legacy' to turn on legacy mode, we would add a new array2string argument like legacy=False. We would also have to keep the style arg.

@eric-wieser
Copy link
Member
eric-wieser commented Oct 3, 2017 •
edited
Loading

FWIW the output in my last comment (without the space) is the output of #9139

In which case it sounds like @lesteve is not using as new a version of numpy as they claim to be?

Either way, I think there are two ways of looking at this:

  • We care about not breaking doctests. Add some more compatibility flags as you suggest.
  • We care about a stupidly long repr of 1e10. In particular, it seems inconsistent that we show far more of the mantissa when the exponent is large (1.13.1): 
     >>> np.array([1.00000000001])
 array([ 1.])
 >>> np.array([1.00000000001e10])
 array([  1.00000000e+10])
    Do we want to throw in a fix for this with the legacy=False mode?

@charris
Copy link
Member
charris commented Oct 3, 2017

My argument for getting all these changes in is

  • Seems lots of developers really want it ;)
  • Warning shot downstream that we won't be held hostage by doctests. Grrr...

In any case, I think we are pretty close to having at least a workable solution for doctests.

ahaldane added a commit to ahaldane/numpy that referenced this issue Oct 3, 2017
@ahaldane
MAINT: create a "legacy" print mode to account for whitespace and 0d …
8e2eee4 
…changes

Fixes numpy#9804
@ahaldane ahaldane mentioned this issue Oct 3, 2017
Merged
@ahaldane
Copy link
Member
ahaldane commented Oct 3, 2017

@eric-wieser, at first glance, I think we shoud just make a separate PR to adjust the precision in the exp notation. It does seem inconsistent and ugly. Hooray for 10.**arange(-5,0) looking nicer!

@lesteve
Copy link
Contributor Author
lesteve commented Oct 3, 2017

In which case it sounds like @lesteve is not using as new a version of numpy as they claim to be?

I am pretty sure that I was using a numpy 1.14 dev version after #9139 was fixed, otherwise I would not have been able to use sign='legacy'.

Maybe we can somehow finagle things so sign='legacy' changes them too, but it seems difficult, but we will think about it.

Warning shot downstream that we won't be held hostage by doctests. Grrr...

As a scikit-learn developer I completely understand this ;-). As I said I think we can work around the problem without requiring numpy changes. I just wanted to report the behaviour so that you can decide what to do about it. And answering "tough luck" is certainly an option I would say, if covering all sign='legacy' edge cases is way too costly.

It's a bit hard for me to keep track of all the PRs related to numpy array str/repr related work but do feel free to ping me and I can test some changes on the scikit-learn doctests if that's helpful.

@ahaldane
Copy link
Member
ahaldane commented Oct 4, 2017

Oh I see the confusion. My output was for sign='-', @lesteve's was for sign='legacy'.

ahaldane added a commit to ahaldane/numpy that referenced this issue Nov 8, 2017
@ahaldane
MAINT: create a "legacy" print mode to account for whitespace and 0d …
7ca029a 
…changes

Fixes numpy#9804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@charris @eric-wieser @lesteve @ahaldane
0