Skip to content
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

OverflowError when raise_on_invalid=True #13

Closed
davidsmf opened this issue Jun 25, 2018 · 3 comments · Fixed by #14
Closed

OverflowError when raise_on_invalid=True #13

davidsmf opened this issue Jun 25, 2018 · 3 comments · Fixed by #14
Labels

Comments

@davidsmf
Copy link

Minimum, Complete, Verifiable Example

from fastnumbers import fast_real

a = fast_real("03e106196") # This is OK
try:
  b = fast_real("03e106196", raise_on_invalid=True) # This throws exception
except ValueError:  # It doesn't get caught, as it's not a ValueError
  pass

Error message, Traceback, Desired behavior, Suggestion, Request, or Question

I would expect a ValueError as it's not a valid float. However, instead there's an OverflowError.

Traceback (most recent call last):
  File "test.py", line 5, in <module>
    b = fast_real("03e106196", raise_on_invalid=True) # This throws exception
OverflowError: cannot convert float infinity to integer

It looks like this is because string_contains_intlike_float returns true for this value, but isn't aware about the exponent being really big.

@SethMMorton
Copy link
Owner

SethMMorton commented Jun 26, 2018

This is a good find!

If this were written as an int (i.e. 3 * 10**106196) then Python would be able to return it as an int just fine without overflow. In this sense, string_contains_intlike_float isn't wrong to return true.

On the other hand, the algorithm being used to return an int from fast_real if parsing the input as an integer fails is to parse as a float, and then convert float output to int. In this case, float("03e106196") is inf, which creates an OverflowError when passed to int.

I think the bug is not in string_contains_intlike_float, but rather in str_to_PyInt_or_PyFloat. Consider the following:

In [2]: import fastnumbers

In [3]: fastnumbers.fast_float("03e106196")
Out[3]: inf

In [4]: fastnumbers.fast_real("03e106196")
Out[4]: '03e106196'

In [5]: fastnumbers.fast_real("03e106196", coerce=False)
Out[5]: inf

In [6]: fastnumbers.fast_real("03e106196", raise_on_invalid=True, coerce=False)
Out[6]: inf

In [7]: fastnumbers.fast_real("03e106196", raise_on_invalid=True)
---------------------------------------------------------------------------
OverflowError                             Traceback (most recent call last)
<ipython-input-7-64eb4b6453ff> in <module>()
----> 1 fastnumbers.fast_real("03e106196", raise_on_invalid=True)

OverflowError: cannot convert float infinity to integer

In [8]: fastnumbers.__version__
Out[8]: '2.0.4'

If fast_float can return a number, fast_real should always be able to return a number, but this is not the case with coerce=True (the default). This is because the string looks like it should be an int but is too big to be stored as a float.

The correct thing to do would be to detect if inf was returned, and in that case do some special handling on the string to ensure an int is returned. I'll give this some thought.

@SethMMorton
Copy link
Owner

SethMMorton commented Jun 26, 2018

In your opinion, what should be the correct behavior?

>>> fastnumbers.fast_real("03e106196") == inf  # coerce=True

or

>>> fastnumbers.fast_real("03e106196") == 3 * 10**106196  # coerce=True

I would expect the same result whether raise_on_invalid is True or False.

@davidsmf
Copy link
Author

oh, good question. Python says "inf" for the literal 3e106196, so I'd go with that.

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

Successfully merging a pull request may close this issue.

2 participants