Skip to content

FLOAT_CMP isn't powerful enough to handle whitespace that is added in fixed-width printing of floats #92

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

Open
ayshih opened this issue Dec 20, 2019 · 5 comments

Comments

@ayshih
Copy link
Contributor

ayshih commented Dec 20, 2019

See title. Here's a minimal example with ndarray, where I vary the least significant digit of the element the first row:

>>> import numpy as np
>>> np.array([[0.12345678], [0.11111111]])
array([[0.12345678],
       [0.11111111]])

>>> np.array([[0.12345671], [0.11111111]])  # doctest: +FLOAT_CMP
array([[0.12345678],
       [0.11111111]])

>>> np.array([[0.12345670], [0.11111111]])  # doctest: +FLOAT_CMP
array([[0.12345678],
       [0.11111111]])

The second test passes with FLOAT_CMP because the change is well under the relative tolerance.
The third test fails with:

Expected:
    array([[0.12345678],
           [0.11111111]])
Got:
    array([[0.1234567 ],
           [0.11111111]])

This failure isn't because of exceeding the tolerance, but rather because of the space between "7" and "]". (I'm hitting this issue with a SkyCoord transformation, where the 32-bit calculation ends in a zero and the 64-bit calculation does not.)

The "obvious" workaround is to turn off FLOAT_CMP and rely on ELLIPSIS to handle the slight discrepancy. But, that could mean adding a lot of ellipsis markers. Is there a better solution?

@ayshih
Copy link
Contributor Author

ayshih commented Dec 20, 2019

I could change the NumPy printing. First setup with:

>>> import numpy
>>> old_floatmode = np.get_printoptions()['floatmode']
>>> np.set_printoptions(floatmode='fixed')

and then teardown with:

>>> np.set_printoptions(floatmode=old_floatmode)

I'll have to mull over whether that's a better workaround.

@bsipocz
Copy link
Member

bsipocz commented Dec 20, 2019

ideally NORMALIZE_WHITESPACE should take care of this, I think. So likely this may be a bug?

@ayshih
Copy link
Contributor Author

ayshih commented Dec 20, 2019

I guess it's technically out of the scope of NORMALIZE_WHITESPACE because that treats all non-zero whitespace the same (and different from zero whitespace), but what I want is for zero whitespace to be treated the same as non-zero whitespace when a comma is involved.

Anyway, I decided that being able to do proper float comparisons is more desirable than using ellipsis markers, so I added (hidden) testsetup/testcleanup blocks as in my earlier comment.

@bsipocz
Copy link
Member

bsipocz commented Dec 20, 2019

I guess it's technically out of the scope of NORMALIZE_WHITESPACE because that treats all non-zero whitespace the same (and different from zero whitespace), but what I want is for zero whitespace to be treated the same as non-zero whitespace when a comma is involved.

yes, thus I think ideally NORMALIZE_WHITESPACE should deal with this, or should at least work with other directives better.

@pllim
Copy link
Contributor

pllim commented Jun 23, 2020

I just hit this very same problem over at astropy/astropy#10511 . Glad I am not going crazy. I also resorted to manually setting numpy print options, which isn't pretty in the user example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants