classification
Title: int() accepts float number base
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.4, Python 3.3, Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: chris.jerdonek, ezio.melotti, georg.brandl, gregory.p.smith, mark.dickinson, python-dev, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2012-12-24 20:10 by serhiy.storchaka, last changed 2013-01-27 16:50 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
issue16772.patch mark.dickinson, 2012-12-28 10:27 review
issue16772_v2.patch mark.dickinson, 2012-12-28 11:04 review
Messages (24)
msg178095 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 20:10
I'm not sure this is a bug. In 2.7 int() and long() with float number base raises TypeError. But in 3.x int() accepts float number base and truncate it to int.
msg178135 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-12-25 15:00
Can you give examples?  I'm unable to guess what exactly you are reporting from quick experiments.
msg178136 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-12-25 15:03
2.7$ ./python -c 'int("5", 12.5)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: integer argument expected, got float
3.2$ ./python -c 'int("5", 12.5)'
3.2$
msg178137 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-12-25 15:08
Ah.  I was thinking of things like ``int('1.2', 10)``, not the base itself being a float.

In this case, looks like a bug to me.
msg178147 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-25 16:29
I agree that this should be fixed.
msg178162 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-25 21:05
New changeset e510e028c486 by Gregory P. Smith in branch 'default':
Fixes issue #16772: int() constructor second argument (base) must be an int.
http://hg.python.org/cpython/rev/e510e028c486
msg178163 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-12-25 21:07
If someone thinks this should go into 3.2 and 3.3 they're welcome to do it; no objections from me.  (The behavior was unintentional and thus a bug, but it is still a minor behavior change)
msg178166 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-25 21:54
A test also needs to be added, though I'm sure one will be added as part of issue 16761.
msg178172 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-25 23:06
Greg:  please could you add a test?

I think that the new check may be too strict:  should we also be prepared to accept any object that implements __index__ as a base?  (Similar to the way that round accepts an __index__-aware object for its second argument.)

<grump> It would have been nice to have a chance to review this change before it was committed. </grump>
msg178185 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-26 06:38
New changeset 60f7197f991f by Gregory P. Smith in branch 'default':
Test for issue16772 and redoes the previous fix to accept __index__-aware
http://hg.python.org/cpython/rev/60f7197f991f
msg178188 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-12-26 06:45
Thanks for the pointer to round(). PyNumber_AsSsize_t was just what the doctor ordered.

PS Grump acknowledged and accepted.  Its trunk and we're nowhere near a freeze so I figured it'd be fine to iterate on it in trunk if needed.
msg178223 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-26 15:11
Why does PyNumber_AsSsize_t() used instead PyLong_AsLongAndOverflow()? The range of "long" enough for all valid values of "base" and code with PyLong_AsLongAndOverflow() produces better error messages. It looks as a regression for me.
msg178259 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-12-26 21:46
I used it because Mark suggested it could have the same behavior as round() and that is the more abstract API that round uses.  Regardless, at this point I've added tests and tested for the TypeError when a float is passed as well as testing that the only valid values (2..36) all work.

I did not test to see that something supporting __index__ works as Mark suggests because I don't know off the top of my head of anything that does and it seems like a pointless thing to care about for this API.

If you want something different, feel free to change the code.  I'm done with this issue.
msg178279 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-27 06:48
But you have "if (!PyLong_Check(obase))" check before. Only ints acceptable. The only difference with previous code is that now OverflowError raised for large bases instead of ValueError. int.__round__ doesn't produce OverflowError.

In any case the final version of those changes should be applied to 3.2 and 3.3 too (if no one objects).
msg178280 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-27 06:49
> In any case the final version of those changes should be applied to 3.2 and 3.3 too (if no one objects).

+1
msg178377 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-28 09:33
I'd suggest leaving 3.2 and 3.3 as they are: the bug is fairly benign, but  fixing it could break existing code unnecessarily.  That's something that we should try hard not to do in a bugfix release.

As to PyNumber_AsSsize_t() used instead PyLong_AsLongAndOverflow(), I *do* think that in general interfaces for built-in functions and methods that accept an integer should be prepared to accept anything that has an __index__.  If we can find a way to do that with sane exception types / messages, so much the better.  (One common application of __index__-able types occurs when using NumPy, where it's easy to end up with instances of numpy.int32 or numpy.int64 instead of regular Python ints.)  I agree with Serhiy that ValueError is the appropriate exception for out-of-range values.

[A side-issue here is that the various PyLong_As* utility functions are a mess: some use __int__, some use __index__, etc.  I have some thoughts about how to fix this, but that's another issue.]
msg178378 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-28 10:11
> The only difference with previous code is that now OverflowError raised > for large bases instead of ValueError.

Serhiy: can you clarify this remark?  Where do you see the OverflowError? The current exception and message look fine to me, so maybe I'm misunderstanding what you're talking about:

Python 3.4.0a0 (default:1b2134a78c17, Dec 28 2012, 10:06:47) 
[GCC 4.2.1 (Apple Inc. build 5664)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> int('34', base=2**100)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: int() base must be >= 2 and <= 36
[66206 refs, 23451 blocks]

I actually think this issue can be closed as fixed:  the current code looks fine to me, and I don't think the fix should be backported.
msg178379 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-28 10:27
> I actually think this issue can be closed as fixed:

Ah, whoops;  I failed to understand Serhiy's comment about the still existing "if (!PyLong_Check(obase))", which does indeed mean that the code *still* doesn't work for __index__-able types.

Here's a fix for that, with tests.
msg178380 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-28 10:36
> I actually think this issue can be closed as fixed:  the current code looks fine to me, and I don't think the fix should be backported.

How about backporting the tests?  In addition to adding tests for the fix, Greg added more comprehensive tests for the existing behavior (i.e. test_int_base_limits()).  Backporting the latter would help prevent regressions from future fixes in earlier versions.

Also, if we don't backport shouldn't there be a version changed in the docs?
msg178381 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-28 10:40
Sure, I don't see any issue with backporting test_int_base_limits;  that has little to do with this issue, though, so shouldn't prevent closing this one.

I'll add a Misc/NEWS entry when I commit;  not sure it's meaningful to add doc changes, since I the 3.2 and 3.3 acceptance of floats is undocumented anyway.  I can and will add a versionchanged entry for the acceptance of __index__-able types, though.
msg178382 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-12-28 11:04
Patch including doc update.
msg178406 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-28 18:02
> Serhiy: can you clarify this remark?  Where do you see the OverflowError?
> The current exception and message look fine to me, so maybe I'm
> misunderstanding what you're talking about:

Sorry, I have been confused (and confuse you) by the variety of PyLong_As* and 
PyNumber_* functions. Now I see PyNumber_AsSsize_t(x, NULL) doesn't raise 
OverflowError but truncates its argument.

The patch LGTM.
msg178460 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-12-29 01:44
To me the doc strongly implies, but does not boldly and baldly say, that base should be an int.

"A base-n literal consists of the digits 0 to n-1, with a to z (or A to Z) having values 10 to 35. The default base is 10. The allowed values are 0 and 2-36. Base-2, -8, and -16 literals can be optionally prefixed with 0b/0B, 0o/0O, or 0x/0X, as with integer literals in code. Base 0 means to interpret exactly as a code literal, so that the actual base is 2, 8, 10, or 16, and so that int('010', 0) is not legal, while int('010') is, as well as int('010', 8)."

I think it should be revised to say "The allowed values are ints 0 and 2-36." (or 'integers' or 'index values'). If it had been that already, the current behavior would clearly be a bug and eligible to be fixed in 3.2/3. As it is, I will not argue with whatever others do.

(I strongly suspect there are other places in the docs where int args are similarly implied but not explicit, but int-ness *is* checked. If someone were to complain about 0.0 being rejected, I expect we would correct the doc, not the code.;-)
msg180758 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-27 10:18
New changeset 79db70bd3188 by Mark Dickinson in branch 'default':
Issue #16772: in int(x, base), non-integer bases must have an __index__ method.
http://hg.python.org/cpython/rev/79db70bd3188
History
Date User Action Args
2013-01-27 16:50:53ezio.melottisetstage: needs patch -> resolved
2013-01-27 10:20:44mark.dickinsonsetresolution: fixed
2013-01-27 10:20:30mark.dickinsonsetstatus: open -> closed
2013-01-27 10:18:05python-devsetmessages: + msg180758
2012-12-29 01:44:39terry.reedysetnosy: + terry.reedy
messages: + msg178460
2012-12-28 18:02:03serhiy.storchakasetmessages: + msg178406
2012-12-28 11:04:50mark.dickinsonsetfiles: + issue16772_v2.patch

messages: + msg178382
2012-12-28 10:40:36mark.dickinsonsetmessages: + msg178381
2012-12-28 10:36:27chris.jerdoneksetmessages: + msg178380
2012-12-28 10:27:41mark.dickinsonsetassignee: serhiy.storchaka -> mark.dickinson
2012-12-28 10:27:20mark.dickinsonsetfiles: + issue16772.patch
keywords: + patch
messages: + msg178379
2012-12-28 10:11:03mark.dickinsonsetmessages: + msg178378
2012-12-28 09:33:10mark.dickinsonsetmessages: + msg178377
2012-12-27 16:49:21gregory.p.smithsetassignee: serhiy.storchaka
2012-12-27 06:49:49chris.jerdoneksetmessages: + msg178280
2012-12-27 06:48:50serhiy.storchakasetmessages: + msg178279
2012-12-26 21:46:33gregory.p.smithsetmessages: + msg178259
2012-12-26 15:11:09serhiy.storchakasetmessages: + msg178223
2012-12-26 10:40:01serhiy.storchakalinkissue16784 dependencies
2012-12-26 06:45:49gregory.p.smithsetmessages: + msg178188
2012-12-26 06:38:41python-devsetmessages: + msg178185
2012-12-25 23:06:13mark.dickinsonsetmessages: + msg178172
2012-12-25 21:54:59chris.jerdoneksetmessages: + msg178166
2012-12-25 21:07:24gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg178163
2012-12-25 21:05:41python-devsetnosy: + python-dev
messages: + msg178162
2012-12-25 16:29:15mark.dickinsonsetnosy: + mark.dickinson
messages: + msg178147
2012-12-25 15:08:28georg.brandlsetmessages: + msg178137
2012-12-25 15:03:35ezio.melottisetmessages: + msg178136
2012-12-25 15:00:55georg.brandlsetnosy: + georg.brandl
messages: + msg178135
2012-12-24 20:22:09chris.jerdoneksetnosy: + chris.jerdonek
2012-12-24 20:11:12ezio.melottisetnosy: + ezio.melotti

type: behavior
stage: needs patch
2012-12-24 20:10:16serhiy.storchakacreate