classification
Title: Fix int(base=X)
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: asvetlov, chris.jerdonek, ezio.melotti, gregory.p.smith, pitrou, python-dev, rhettinger, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2012-12-24 10:16 by serhiy.storchaka, last changed 2013-01-02 12:36 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
int_without_x.patch serhiy.storchaka, 2012-12-24 10:16 Patch for 3.3+ review
int_without_x-3.2.patch serhiy.storchaka, 2012-12-24 10:17 Patch for 3.2 review
int_without_x-2.7.patch serhiy.storchaka, 2012-12-24 10:19 Patch for 2.7 review
int_without_x-3.3_2.patch serhiy.storchaka, 2012-12-24 20:22 review
int_without_x-3.2_2.patch serhiy.storchaka, 2012-12-24 20:22 review
int_without_x-2.7_2.patch serhiy.storchaka, 2012-12-24 20:22 review
int_without_x-2.7_3.patch serhiy.storchaka, 2012-12-26 10:47 review
int_without_x-3.2_3.patch serhiy.storchaka, 2012-12-26 10:47 review
int_without_x-3.3_3.patch serhiy.storchaka, 2012-12-26 10:47 review
Messages (19)
msg178043 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 10:16
Currently int() ignores base keyword argument if first string argument omitted. Such usage is no other than bug. The proposed patch raises TypeError in such case.

See also discussion at Python-Dev: http://comments.gmane.org/gmane.comp.python.cvs/92290 .
msg178044 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 10:19
In additional some int creation tests backported to old Python versions.
msg178097 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-24 20:22
Patches updated to address Cris's  comments on Rietveld (thank you, Cris, for 
review). In additional new tests added. I think merging test_int and test_long 
is hard task and should be done in separated issue. Instead I have merged and 
simplified some test to make synchronization simpler.

Two new possible bugs were found during working on these tests (issue16772 and 
issue16773).
msg178100 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-24 21:13
This is good work.  Thanks.

> I think merging test_int and test_long is hard task and should be done in separated issue.

I meant "share" rather than "merge."  For the tests you're adding, isn't it simply a matter of putting the test cases into a separate mixin or TestCase class in a different module, and then importing and subclassing that (and setting the type to use as an attribute of the subclass)?  Or am I missing something?
msg178186 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-12-26 06:43
Having just looked added something to test_int as part of issue16772... There appears to be an explicit test _for_ this strange behavior in there:

http://hg.python.org/cpython/file/60f7197f991f/Lib/test/test_int.py#l233

test_base_arg_with_no_x_arg

I have no idea why this exists or was desired.
msg178187 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-26 06:45
See this thread:

http://mail.python.org/pipermail/python-dev/2012-December/123283.html
msg178192 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-26 09:15
Sharing only those tests is not enough. We must revise all tests and extract common code to the separated class. I think this is too large change and unrelated to this issue. I will open a new issue for int tests enhancement and refactoring.
msg178193 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-26 09:40
It's enough not to make the problem worse, which is why I suggested it and what I wanted to avoid.  If you copy-paste the tests you're adding now, it worsens the problem and makes it that much more tedious to fix later on (and to maintain in the meantime).

Why must the code sharing be all or nothing?  Elsewhere we phase in improvements incrementally.  Even with the particular refactoring you're suggesting for the future, I'm not sure we'd want to do it all in one patch.

If you begin the process now and add appropriate comments and TODO's pointing the way, at least the next person will know how to take the next steps.
msg178198 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-26 10:47
Here is a patches which contains only minimal set of required changes. Other 
unrelated changes will be done in issue16784.
msg178199 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-26 10:57
Let's eat an elephant piece by piece. Only one issue per patch.
msg178202 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-26 11:22
Yes, it is a better approach.  At first glance, the patches look okay to me.
msg178338 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-27 21:16
When this patch is updated because of the commit for issue 16790, in 3.x can the edited test cases be moved to the top of the test class per the following comment (as appropriate)?

http://bugs.python.org/issue16790#msg178282

As stated there, this will make synching tests between 2.7 and 3.x easier.  We can do the others in 3.x whenever more are added to the 2.7 common tests.
msg178371 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-12-28 07:29
I don't think a backport is appropriate for this kind of change.
msg178373 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-28 08:20
New changeset c0266ba8e4c6 by Serhiy Storchaka in branch '2.7':
Issue #16761: Raise TypeError when int() or long() called with base argument only.
http://hg.python.org/cpython/rev/c0266ba8e4c6

New changeset e4ea38a92c4d by Serhiy Storchaka in branch '3.2':
Issue #16761: Raise TypeError when int() called with base argument only.
http://hg.python.org/cpython/rev/e4ea38a92c4d

New changeset 157ff02bcc16 by Serhiy Storchaka in branch '3.3':
Issue #16761: Raise TypeError when int() called with base argument only.
http://hg.python.org/cpython/rev/157ff02bcc16

New changeset 1b2134a78c17 by Serhiy Storchaka in branch 'default':
Issue #16761: Raise TypeError when int() called with base argument only.
http://hg.python.org/cpython/rev/1b2134a78c17
msg178374 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-28 08:28
> When this patch is updated because of the commit for issue 16790, in 3.x can the edited test cases be moved to the top of the test class per the following comment (as appropriate)?

I will do this in issue16784. In any case I am going to move a large part of test_basic/test_long to IntLongCommonTests and then those test will be above test_keyword_args.
msg178375 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-12-28 09:08
Why did you backport this change after being advised not to do it?
msg178402 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-28 17:45
Sorry, I have not noticed your advice before I did commit (it took me a lot of time). Should I now revert my changes to 2.7, 3.2 and 3.3?
msg178414 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-12-28 18:58
If PyPy gets away with a different behaviour, there's probably nobody relying on it, so I'd say the backports are safe.
msg178427 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-12-28 21:11
> Should I now revert my changes to 2.7, 3.2 and 3.3?

Once committed, I would say leave it in as long as you're confident that no currently working code will break as a result.  In general, be very conservative about backporting any behavioral change.
History
Date User Action Args
2013-01-02 12:36:46serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-12-28 21:11:50rhettingersetmessages: + msg178427
2012-12-28 18:58:19pitrousetnosy: + pitrou
messages: + msg178414
2012-12-28 17:45:45serhiy.storchakasetmessages: + msg178402
2012-12-28 09:08:48rhettingersetmessages: + msg178375
2012-12-28 08:28:16serhiy.storchakasetmessages: + msg178374
2012-12-28 08:20:59python-devsetnosy: + python-dev
messages: + msg178373
2012-12-28 07:29:45rhettingersetnosy: + rhettinger

messages: + msg178371
versions: - Python 2.7, Python 3.2, Python 3.3
2012-12-28 06:59:30serhiy.storchakasetassignee: serhiy.storchaka
2012-12-27 21:16:16chris.jerdoneksetmessages: + msg178338
2012-12-26 11:22:03chris.jerdoneksetmessages: + msg178202
2012-12-26 10:57:19serhiy.storchakasetmessages: + msg178199
2012-12-26 10:47:49serhiy.storchakasetfiles: + int_without_x-2.7_3.patch, int_without_x-3.2_3.patch, int_without_x-3.3_3.patch

messages: + msg178198
2012-12-26 10:40:01serhiy.storchakalinkissue16784 dependencies
2012-12-26 09:40:47chris.jerdoneksetmessages: + msg178193
2012-12-26 09:15:28serhiy.storchakasetmessages: + msg178192
2012-12-26 06:45:32chris.jerdoneksetmessages: + msg178187
2012-12-26 06:43:36gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg178186
2012-12-24 21:13:40chris.jerdoneksetmessages: + msg178100
2012-12-24 20:22:07serhiy.storchakasetfiles: + int_without_x-3.3_2.patch, int_without_x-3.2_2.patch, int_without_x-2.7_2.patch

messages: + msg178097
2012-12-24 10:24:34ezio.melottisetnosy: + ezio.melotti
2012-12-24 10:19:55serhiy.storchakasetfiles: + int_without_x-2.7.patch

messages: + msg178044
2012-12-24 10:17:59serhiy.storchakasetfiles: + int_without_x-3.2.patch
nosy: + terry.reedy, asvetlov, chris.jerdonek
2012-12-24 10:16:19serhiy.storchakacreate