classification
Title: Extended slicing with classic class behaves strangely
Type: behavior Stage:
Components: Interpreter Core Versions: Python 2.7, Python 2.6
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: flox, mark.dickinson, rhettinger
Priority: normal Keywords: patch

Created on 2009-12-17 16:54 by flox, last changed 2010-01-10 12:01 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
case_slice.py flox, 2009-12-17 16:54 Buggy behaviour of classic classes
issue7532_classic_getitem_v2.diff flox, 2009-12-18 00:18 Patch, apply to trunk
issue7532_wontfix_tests.diff flox, 2010-01-07 21:30 Patch, apply to trunk
issue7532_bytearray.diff flox, 2010-01-09 22:59 Patch, apply to trunk
issue7532_wontfix_tests_py3k.diff flox, 2010-01-09 23:09 Patch, apply to py3k
Messages (12)
msg96520 - (view) Author: Florent Xicluna (flox) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2010-01-03 09:57
Closing as won't fix.
msg97376 - (view) Author: Florent Xicluna (flox) * (Python committer) 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) * (Python committer) Date: 2010-01-07 21:36
Okay, sounds reasonable.  Reopening to consider tests.
msg97467 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2010-01-10 12:01
Thanks!  Applied in r77408.
History
Date User Action Args
2010-01-10 12:01:12mark.dickinsonsetstatus: open -> closed

messages: + msg97511
2010-01-09 23:09:15floxsetfiles: + issue7532_wontfix_tests_py3k.diff
2010-01-09 23:08:53floxsetfiles: - issue7532_wontfix_tests_py3k.diff
2010-01-09 22:59:48floxsetfiles: + issue7532_bytearray.diff
2010-01-09 22:59:26floxsetfiles: + issue7532_wontfix_tests_py3k.diff

messages: + msg97475
2010-01-09 18:55:03mark.dickinsonsetmessages: + msg97467
2010-01-07 21:36:24mark.dickinsonsetstatus: closed -> open
assignee: mark.dickinson
messages: + msg97379
2010-01-07 21:30:58floxsetfiles: + issue7532_wontfix_tests.diff

messages: + msg97376
2010-01-03 09:57:42mark.dickinsonsetstatus: open -> closed
resolution: wont fix
messages: + msg97157
2009-12-19 12:03:25mark.dickinsonsetmessages: + msg96601
2009-12-18 00:19:28floxsetfiles: - issue7532_classic_getitem.diff
2009-12-18 00:18:01floxsetfiles: + issue7532_classic_getitem_v2.diff

messages: + msg96542
2009-12-17 22:03:12floxsetfiles: + issue7532_classic_getitem.diff
keywords: + patch
messages: + msg96536
2009-12-17 20:42:16mark.dickinsonsetmessages: + msg96533
2009-12-17 20:36:50mark.dickinsonsetnosy: + rhettinger, mark.dickinson
messages: + msg96532
2009-12-17 16:54:08floxcreate