Issue624807
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2002-10-17 18:18 by nascheme, last changed 2022-04-10 16:05 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
bug.py | nascheme, 2002-10-17 18:18 | |||
str_add_mul.diff | nascheme, 2002-11-18 16:29 | Add nb_add nb_mul to str and unicode | ||
fix_inplace_seq.diff | nascheme, 2002-12-22 02:58 |
Messages (13) | |||
---|---|---|---|
msg12806 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-10-17 18:18 | |
See attached code 'bug.py'. What's happening is if the LHS of an inplace add has a sq_concat or sq_inplace_concat method but it cannot handle the RHS argument then the operation fails rather than trying __radd__. I think this is related to my str.__mod__ bug. Guido, I would appreciate any comments you have on fiixing this problem. Fixing this specific problem should be pretty straight forward but I suspect more tp_as_sequence vs. tp_as_number confusion exists. What's the the longterm strategy WRT sq_concat and sq_repeat? |
|||
msg12807 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2002-10-17 18:53 | |
Logged In: YES user_id=6380 Boy what a mess indeed! Most embarrassingly, this code worked in Python 2.0! :-( I'm not sure how to fix this (and have no time to really look into it), but I have a long-term comment. Long-term, at the C API level, the separate notions of "sequence repeat" and "sequence concat" should disappear, and instead "PyNumber_Multiply" and "PyNumber_Add" should be used. (PySequence_Repeat can stay as a shorthand that takes a C int rather than an object). In general, whenever two operations look the same at the python level, there should only be one C API to invoke that, not two. Other examples are PySequence_GetItem vs. PyObject_GetItem. As a transition, the built-in types should probably start supporting the more generic ops (so strings would support nb_add and nb_mul), and the generic API functions should look for the numeric ops before trying the sequence or mapping ops. (And also try the mapping ops before the -- more restrictive -- sequence ops.) Some of this is already going on, but it would be a good policy to try and fix all of this in 2.3. I expect it wouldn't break much -- you'd have to have a type that interprets e.g. sq_repeat different than nb_mul to get code breakage. |
|||
msg12808 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-10-25 21:27 | |
Logged In: YES user_id=35752 I got a bit worried when test_select failed after adding tp_as_number to the str object. It was triggered by select.select([], [], [], 'not a string'). The source of the problem was (hope this comes out okay): else if (!PyNumber_Check(tout)) { PyErr_SetString(PyExc_TypeError, "timeout must be a float or None"); return NULL; } else { tout = PyNumber_Float(tout); if (!tout) return NULL; The test was expecting TypeError but getting ValueError. I guess PyNumber_Check is pretty useless. Luckily it is only used in the select and operator modules. |
|||
msg12809 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-11-18 16:29 | |
Logged In: YES user_id=35752 The attached patch adds nb_add and nb_mul slots to str and unicode. All tests pass after this change. One resulting behavior change is that: "ab" * 4.5 works like "ab" * 4 That's a little unsettling to me but I guess it's consistient with: "%d" % 4.5 Please review. |
|||
msg12810 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-11-18 16:32 | |
Logged In: YES user_id=35752 The attached patch adds nb_add and nb_mul slots to str and unicode. All tests pass after this change. One resulting behavior change is that: "ab" * 4.5 works like "ab" * 4 That's a little unsettling to me but I guess it's consistient with: "%d" % 4.5 Please review. |
|||
msg12811 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2002-11-18 18:04 | |
Logged In: YES user_id=6380 Haven't looked at the patch, but I find that unacceptable. "%d" is sort of acceptable (although I wish it would round); there are a few other places where floats are accidentally accepted, but all of those are mistakes and we shouldn't add any more of those. Now what? |
|||
msg12812 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-11-18 19:54 | |
Logged In: YES user_id=35752 Well, perhaps we cannot (eventually) get rid of sq_repeat. I actually quite like the way things work now. Just to refresh your memory in case it's fuzzy, the __mul__ method of integer like objects check for sq_repeat on the other object. If it exists then it calls it with itself as the "count" argument. Sequences don't implement nb_mul or return NotImplemented. So, the "protocol" has two parts. Sequence types expose a sq_repeat slot and integer types have a nb_mul that checks for sq_repeat. It's unclear to me what a __mul__ method on a sequence could do. What types of integer arguments should it accept? Allowing only ints and longs seems wrong to me. It should be possible for a extension type to work as well. I just noticed that the type __mul__ is wierd: >> 'a'.__mul__(3.4) 'aaa' >>> [1].__mul__(3.4) [1, 1, 1] To make it cosistent with the current modus operandi it should call __mul__ on the argument: >>> (3).__mul__('a') 'aaa' >>> (3.3).__mul__('a') NotImplemented |
|||
msg12813 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2002-11-18 20:00 | |
Logged In: YES user_id=6380 Sounds about right -- time to clise this issue? Note that the sq_repeat slot takes a C int, which somewhat limits what you can do with it, but that doesn't feel like a big problem. Can you figure out why 'a'.__mul__(3.5) does what it does? That seems like a bug to me. |
|||
msg12814 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-12-22 02:58 | |
Logged In: YES user_id=35752 The attached patch fixes the orignal bug reported along with a number of other related bugs. The patch does two things. First, it rearranges the code in abstract.c so that sq_repeat, sq_inplace_repeat, sq_concat and sq_inplace_concat are always called after trying the nb_* slots. The code before the patch was inconsistent about this and that caused the reported bug as well as others. The patch also consolidates the int and long sequence repeat code. Before the change, integers checked for overflow but longs did not. The consolidated code removes duplication and gets rid of some ugly code in intobject.c and longobject.c. The bug related to type.__mul__ has not been fixed. I'll open a new bug for it rather than reusing this one. Guido, I think this solves all the issues I dicussed with you a while back on the phone. It's amazing what you can get done on a long, cross country flight. :-) |
|||
msg12815 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-12-30 15:28 | |
Logged In: YES user_id=35752 This should probably get in before the alpha release. Guido appears to be busy. Feel free to reassign if you don't have time. |
|||
msg12816 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2002-12-30 15:50 | |
Logged In: YES user_id=6380 I'll look at it right now. |
|||
msg12817 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2002-12-30 16:19 | |
Logged In: YES user_id=6380 This looks good. Neil, do you have time to commit the changes today? If not, reassign to me and I'll do it. |
|||
msg12818 - (view) | Author: Neil Schemenauer (nascheme) * | Date: 2002-12-30 20:31 | |
Logged In: YES user_id=35752 Checked in. See abstract.c 2.113 for the main change. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-10 16:05:45 | admin | set | github: 37334 |
2002-10-17 18:18:25 | nascheme | create |