| msg96520 - (view) |
Author: Florent Xicluna (flox) *  |
Date: 2009-12-17 16:54 |
| See attached example. The Classic class should behave like the New-style class. |
|
|
| msg96532 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-12-17 20:36 |
| Hmm. This doesn't look like something that's easy to fix without affecting existing correct code; given that the behaviour has been around for a while (I'm not sure exactly how long), and that the issue is gone in 3.x, I suspect it may not be worth trying. Analysis: for classic classes, the ceval loop calls PySequence_GetSlice, which corrects the negative indices by adding the length of the sequence (as described in the docs) and then calls the tp_as_sequence->sq_slice slot on the type. That ends up calling instance_slice (in Objects/classobject.c), which discovers that the class doesn't implement __getslice__ and so passes the adjusted slice on to the __getitem__ method. I'm not sure how this could be changed to get the correct behaviour: PySequence_GetSlice would somehow need to know that it was going to end up calling __getitem__ rather than __getslice__. Raymond, any thoughts? |
|
|
| msg96533 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-12-17 20:42 |
| Issue #974635 looks like the same thing. That issue was closed as a duplicate of issue #723806, though to my eyes #723806 doesn't look quite the same. |
|
|
| msg96536 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2009-12-17 22:03 |
| Mark, Thank you for your analysis. I looked at similar implementation of tp_as_sequence->sq_slice slots in "stringobject.c" (and tuple, list). I've added extra controls before the _PySlice_FromIndices call to let it behave like new-style classes. I have updated the tests. It should be safe for 2.7, no? |
|
|
| msg96542 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2009-12-18 00:18 |
| Patch augmented with extensive tests: * Classic class or New-style class * with or without __getslice__ |
|
|
| msg96601 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-12-19 12:03 |
| Interesting solution! While the patch itself looks fine to me, I'm not sure I like this solution much. It's fine to use this trick for list or tuple, but implementing it for all old-style classes at once seems a bit dangerous. With this patch, it seems to me that the rule describing exactly what __getitem__ receives (for an old-style class implementing __getitem__ but not __getslice__) becomes rather complicated, and can no longer be deduced from the documentation. I'd say leave the current behaviour as it is, and remind people that they should be using new-style classes wherever possible. |
|
|
| msg97157 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-01-03 09:57 |
| Closing as won't fix. |
|
|
| msg97376 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2010-01-07 21:30 |
| I would suggest to keep the tests, even if the bug is closed. |
|
|
| msg97379 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-01-07 21:36 |
| Okay, sounds reasonable. Reopening to consider tests. |
|
|
| msg97467 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-01-09 18:55 |
| Tests applied to trunk in r77391. Are you interested in producing a py3k version of the patch? |
|
|
| msg97475 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2010-01-09 22:59 |
| Here it is, with some cleaning and simple Bytes/Bytearray tests. And Bytearray tests backported to 2.7. |
|
|
| msg97511 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-01-10 12:01 |
| Thanks! Applied in r77408. |
|
|