Stefan Krah added the comment:
OverflowError during initialization dates back to the early days of Python:
http://hg.python.org/cpython/rev/0437738279a8
Integer arithmetic actually did raise OverflowError back then:
2 * 22
Traceback (most recent call last):
File
Mark Dickinson added the comment:
Looks like this harmless-looking issue has opened up a can of worms.
the whole idea of OverflowError seems slightly outdated.
Well, not entirely. It's still got a place for overflow of mathematical
operations, and I think it's clearly the correct exception
Stefan Krah added the comment:
Mark Dickinson rep...@bugs.python.org wrote:
the whole idea of OverflowError seems slightly outdated.
Well, not entirely. It's still got a place for overflow of mathematical
operations, and I think it's clearly the correct exception in that case
(indeed,
Roundup Robot added the comment:
New changeset 273e17260d25 by Mark Dickinson in branch 'default':
Issue #20539: Improve math.factorial error messages and types for large inputs.
http://hg.python.org/cpython/rev/273e17260d25
--
nosy: +python-dev
___
Mark Dickinson added the comment:
Patch applied to the default branch (but with OverflowError instead of
ValueError for large positive inputs). I don't see a lot of benefit in
applying the fix to the maintenance branches. Closing this issue.
--
resolution: - fixed
status: open -
Mark Dickinson added the comment:
Updated patch.
--
Added file: http://bugs.python.org/file34777/huge_factorial_input_v3.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
Changes by Mark Dickinson dicki...@gmail.com:
--
assignee: - mark.dickinson
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
___
Josh Rosenberg added the comment:
Mark, you said:
OverflowError seems to have the majority vote. I'll change it to that.
But your most recent patch seems to have gone back to ValueError for both
cases. Is there a reason for that? FWIW, I favor OverflowError as the too
large type, though I
Mark Dickinson added the comment:
Josh: well spotted; thanks for picking up on this. I was wondering whether
anyone would pick up on that, and I should have mentioned the change explicitly.
I do think ValueError is the correct exception here. My interpretation of
OverflowError is that it
Josh Rosenberg added the comment:
I just think it's a little odd to make math.factorial uniquely sensitive to the
documented definition of OverflowError. Basically every one of the non-masking
PyLong_AsCType interfaces uses OverflowError for too large values.
In fact, all the functions which
Mark Dickinson added the comment:
Making math.factorial the exception would be violating the reasonable
expectation one might get from using similar functions.
Can you point to some examples of those similar functions? I can't think of
any obvious examples offhand.
--
Josh Rosenberg added the comment:
A few examples (some are patently ridiculous, since the range of values anyone
would use ends long before you'd overflow a 32 bit integer, let alone a 64 bit
value on my build of Python, but bear with me:
datetime.datetime(2**64, 1, 2)
Traceback (most recent
New submission from Nick Coghlan:
I believe this is mostly a curiousity (since actually calculating a factorial
this big would take an interminable amount of time), but math.factorial can be
provoked into throwing OverflowError by a large enough input:
math.factorial(10**19)
Traceback (most
Changes by Nick Coghlan ncogh...@gmail.com:
--
priority: normal - low
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
___
Python-bugs-list
Mark Dickinson added the comment:
What behaviour would you suggest instead?
Apart from the time, the result would have over 10**20 digits, so would need
exabytes of memory to represent.
from math import lgamma, log
lgamma(1e19) / log(10)
1.8565705518096748e+20
--
nosy:
STINNER Victor added the comment:
What behaviour would you suggest instead?
Some others Python functions like str * int raises a MemoryError on overflow.
But I don't know if OverflowError or MemoryError is better.
--
nosy: +haypo
priority: low - normal
Mark Dickinson added the comment:
Some others Python functions like str * int raises a MemoryError on overflow.
I have to say that that's always seemed wrong to me: IMO MemoryError should
only be raised when we've really run out of memory, or perhaps where the amount
of memory we're about to
Stefan Krah added the comment:
OverflowError seems like a good choice if only values in the range
of a C long are accepted. ValueError would perhaps be more intuitive,
since the user normally only cares about the accepted range of
input values rather than the internal details.
--
nosy:
Nick Coghlan added the comment:
I figured it was some internal storage overflowing, and I don't have an issue
with the error type. The bit that is confusing is the error *message* - what
int doesn't fit into a C long? (especially in the second case of float input
where no user level int is
Mark Dickinson added the comment:
The bit that is confusing is the error *message* - what int doesn't fit into
a C long?
Agreed: it would be nice to intercept this and give a better message.
--
___
Python tracker rep...@bugs.python.org
Antoine Pitrou added the comment:
I also think OverflowError is better than MemoryError here.
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
Gareth Rees added the comment:
It's not a case of internal storage overflowing. The error is from
Modules/mathmodule.c:1426 and it's the input 10**19 that's too large
to convert to a C long. You get the same kind of error in other
places where PyLong_AsLong or PyLong_AsInt is called on a
Changes by Mark Dickinson dicki...@gmail.com:
--
components: +Extension Modules
type: - behavior
versions: +Python 3.4
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
STINNER Victor added the comment:
OverflowError makes sense because math.factorial(10**19) will overflow in
CPython on common platforms, even if it didn't overflowed yet.
On a supercomputer with a different Python implementation, you may be able to
compute it.
IMO An OverflowError is
Mark Dickinson added the comment:
Okay; OverflowError seems to have the majority vote. I'll change it to that.
Any strong opinions on the message?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
Antoine Pitrou added the comment:
Outrageously sounds like the argument is immoral, but do we have an objective
test for that? :-)
(nitpicking a bit: negative values should probably raise a proper ValueError,
no?)
--
___
Python tracker
Mark Dickinson added the comment:
nitpicking a bit: negative values should probably raise a proper ValueError,
no?
I think they do, with this patch. Or maybe I'm misunderstanding?
With the current form of the patch:
math.factorial(10**20)
Traceback (most recent call last):
File stdin,
Antoine Pitrou added the comment:
Ah, well, my bad. I just wanted to have a good argument :-)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
Mark Dickinson added the comment:
Updated patch.
--
Added file: http://bugs.python.org/file33965/huge_factorial_input_v2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20539
___
Mark Dickinson added the comment:
Thinking about it, `ValueError` seems like the right exception type: nothing's
actually overflowing, because we haven't even tried to do any computation.
This is just a limitation of what `math.factorial` is prepared to accept as
input (and perhaps that
Stefan Krah added the comment:
I slightly favor the ValueError patch because there is only a single exception
to catch. PyLong_AsUnsignedLong() also raises OverflowError for both positive
values that are too large and for negative values.
Perhaps the error message could contain the actual range
Gary Bernhardt added the comment:
Although I thoroughly enjoyed outrageously, I agree with Stefan about
including the range of values. As a user who doesn't know the implementation,
outrageously will just leave me asking why, but indicating the range will
tell me exactly why the exception was
32 matches
Mail list logo
