| msg178095 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
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) * |
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) * |
Date: 2012-12-25 15:03 |
| 2.7$ ./python -c 'int("5", 12.5)' Traceback (most recent call last): File "", line 1, in TypeError: integer argument expected, got float 3.2$ ./python -c 'int("5", 12.5)' 3.2$ |
|
|
| msg178137 - (view) |
Author: Georg Brandl (georg.brandl) * |
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) * |
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) * |
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) * |
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) * |
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.) It would have been nice to have a chance to review this change before it was committed. |
|
|
| 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 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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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 "", line 1, in 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) * |
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) * |
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) * |
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) * |
Date: 2012-12-28 11:04 |
| Patch including doc update. |
|
|
| msg178406 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
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) * |
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 |
|
|