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

math.isnan(int) and math.isinf(int) should not raise OverflowError #72162

Closed
stevendaprano opened this issue Sep 6, 2016 · 11 comments
Closed
Labels
type-bug An unexpected behavior, bug, or error

Comments

@stevendaprano
Copy link
Member

stevendaprano commented Sep 6, 2016

BPO 27975
Nosy @tim-one, @mdickinson, @stevendaprano

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-09-07.18:28:12.010>
created_at = <Date 2016-09-06.17:04:33.306>
labels = ['type-bug']
title = 'math.isnan(int) and math.isinf(int) should not raise OverflowError'
updated_at = <Date 2016-09-07.18:28:12.008>
user = 'https://github.com/stevendaprano'

bugs.python.org fields:

activity = <Date 2016-09-07.18:28:12.008>
actor = 'mark.dickinson'
assignee = 'none'
closed = True
closed_date = <Date 2016-09-07.18:28:12.010>
closer = 'mark.dickinson'
components = []
creation = <Date 2016-09-06.17:04:33.306>
creator = 'steven.daprano'
dependencies = []
files = []
hgrepos = []
issue_num = 27975
keywords = []
message_count = 11.0
messages = ['274568', '274571', '274572', '274573', '274578', '274580', '274586', '274592', '274596', '274598', '274859']
nosy_count = 3.0
nosy_names = ['tim.peters', 'mark.dickinson', 'steven.daprano']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue27975'
versions = []

@stevendaprano
Copy link
Member Author

stevendaprano commented Sep 6, 2016

Currently, math.isnan(n) and math.isinf(n) for n an int may raise OverflowError if n is too big to convert to a float, e.g.:

py> math.isnan(10**10000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: int too large to convert to float

But this conversion is unnecessary. int does not support either INF or NAN, so there is no need to convert the value to float first. If the argument is an int, the result must be False.

The same applies to Fraction.

@mdickinson
Copy link
Member

mdickinson commented Sep 6, 2016

I see this as a documentation issue: the vast majority of math module functions are designed to operate on floats, and if given a non-float input, simply convert that input to a float as a convenience. If we start special-casing math module functions for int, Fraction, and Decimal inputs, the module is going to become much more complicated both in terms of implementation and in terms of cognitive load for the user.

@mdickinson
Copy link
Member

mdickinson commented Sep 6, 2016

Related: bpo-18842

@stevendaprano
Copy link
Member Author

stevendaprano commented Sep 6, 2016

As a convenience for whom? Certainly not the poor user, who thinks that
math.isnan(x) should return False if the number x is not a NAN. Since
neither 10**1 nor 10**100000 are NANs, why should one return correctly
and the other raise a completely spurious OverflowError?

I cannot speak for the implementation, except as a wild guess. It
shouldn't be hard to do the equivalent of:

if type(x) == int: return False  # Intentionally excluding subclasses.
try:
    y = float(x)
except OverflowError:
    return False
else:
    ... # existing implementation

but since I know nothing about the C implementation maybe I'm completely
wrong. But as far as the user's cognitive load, I don't think that:

"math.isnan(x) might return the expected result, or it might raise
OverflowError"

is *simpler* for the user than:

"math.isnan(x) will return the expected result".

@mdickinson
Copy link
Member

mdickinson commented Sep 6, 2016

It shouldn't be hard to do the equivalent of:

Right, that's not hard at all. But is it what we want to do? Why do you single out int for special treatment, but not Fraction or Decimal? How should the implementation handle Fraction objects, and why? How should the implementation handle a Fraction-like object implemented by a user? Why only objects of exact type int, but not instances of subclasses? Your suggestion replaces a straightforward mental model (math.isnan converts its input to float, then operates on it, just like almost all other math module functions) with something more complicated.

-1 from me.

@mdickinson
Copy link
Member

mdickinson commented Sep 6, 2016

As a convenience for whom?

I was referring to the general math module model. Being able to type sqrt(2) rather than having to type sqrt(float(2)) or sqrt(2.0) is a convenience.

@tim-one
Copy link
Member

tim-one commented Sep 6, 2016

The only sane way to do things "like this" is to allow types to define their own special methods (like __isnan__()), in which case the math module defers to such methods when they exist. For example, this is how math.ceil(Fraction) works, by deferring to Fraction.__ceil__(). The math module itself knows nothing else about what ceil(Fraction) could possibly mean. All it knows is "if the type has ceil use that, else convert to float first".

I'm also -1 on adding masses of if/else if/else if/.../else constructs to the math module to build in knowledge of the builtin numeric types. Do it "right" or not at all.

I'd just be -0 on adding masses of new __isnan__, __isinf__, ..., special methods. They're just not useful enough often enough.

@stevendaprano
Copy link
Member Author

stevendaprano commented Sep 6, 2016

On Tue, Sep 06, 2016 at 05:59:08PM +0000, Mark Dickinson wrote:

Why do you single out int for special treatment,

Mostly as a demonstration for what could be done, not necessarily as
what should be done. Secondly as a potential optimization. Why go to the
expense of converting something to a float when there's a much
cheaper(?) test?

but not Fraction or Decimal?

They're not built-ins. They would have to be imported first, before you
can test for their types. That could be costly, and it would rarely be
necessary.

How should the implementation handle Fraction objects, and why?

If some hypothetical subclass of Fraction provides a NAN or INF value,
trust that float(x) of those values will return a float NAN or INF. If
the conversion overflows, the value isn't a NAN or INF.

How should the implementation handle a Fraction-like object
implemented by a user?

As above.

Why only objects of exact type int, but not instances of subclasses?

Subclass of int might hypothetically implement their own NAN or INF
values. In which case, trust that MyInt('nan').__float__() will return a
NAN float as it is supposed to.

Your suggestion replaces a straightforward
mental model (math.isnan converts its input to float, then operates on
it, just like almost all other math module functions) with something
more complicated.

Not more complicated. An even more simple *model*. The existing model
for (let's say isnan):

"Convert the number x to a float, which may Overflow, then return
whether the float is a NAN."

Versus the even simpler model:

"Return whether the number x is a NAN."

(Which of course hides a more complex *implementation*.)

Versus the practice of pushing the complexity onto the users:

"Oh gods, what sort of number is my x? If I pass it to math.isnan, will
it blow up? Better wrap it in a try...except ValueError just in case.
[Later] Ah, dammit, I meant OverflowError! Oh no, is there an
UnderflowError for Fractions?"

I guess the argument boils down to whether we want to prioritise
simplicity or usefulness in the math module.

@mdickinson
Copy link
Member

mdickinson commented Sep 6, 2016

[Steven]

Versus the even simpler model:
"Return whether the number x is a NAN."

But what you're proposing doesn't match that description! Under your proposal, math.isnan(10**1000) would be False, but math.isnan(Fraction(10**1000)) would again raise an OverflowError, as would math.isnan(Decimal('1e500')).

@mdickinson
Copy link
Member

mdickinson commented Sep 6, 2016

as would math.isnan(Decimal('1e500'))

Whoops, no. I'd forgotten that large finite Decimal objects end up as float infinities under conversion. Not sure I like that much, but it is what it is ...

@mdickinson
Copy link
Member

mdickinson commented Sep 7, 2016

Closing this as "won't fix". I agree with Tim that the right way to handle this is to make math.isnan behave like math.floor and math.ceil currently do, via new special methods, but (1) I think introducing new special methods should probably be a PEP-level change, and (2) I'm afraid I'm not motivated enough to implement it, so it's going to have to wait on someone who has that motivation.

@mdickinson mdickinson added the type-bug An unexpected behavior, bug, or error label Sep 7, 2016
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants