[C++-sig] Re: New slice implementation

[Raoul Gough]

Jonathan Brandmeyer <jbrandmeyer at earthlink.net> writes:

> On Tue, 2004-01-06 at 11:09, Raoul Gough wrote:
>> Jonathan Brandmeyer <jbrandmeyer at earthlink.net> writes:
>> 
>> > I have written a new implementation for free-standing slice objects. 
>> > This allows you to create and query slice objects that include a
>> > step-size, as well as export a form of __getitem__ and/or __setitem__
>> > that can receive slice arguments and tuples of slice arguments.
>> 
>> Not sure how much overlap there is, 
>
> It turns out that there isn't much.  slice::get_indicies() overlaps with
> the indexing suite's slicing support, but in a different way such that
> it might be OK to keep, anyway.  My object really just provides a object
> manager for PySliceObject, rather than provide a container of objects
> initialized by a slice (such as the indexing_suite does).

It sounds like the interfaces may be quite different, but isn't the
functionality the same? Once you have a Python slice object, you can
use this to access portions of a container via __getitem__ from the
indexing suite. On the other hand, if the slice object is actually one
of your python::slice objects, you could use its get_indices member
function to access parts of a container as well. I guess the main
difference is whether this is returned via a separate container, or
via iterators into the existing container. Note the potential problems
from the Python side, though, if the existing container disappears
while those iterators still exist.

BTW, wouldn't it be a good idea to have a slice constructor that takes
a PySliceObject * as parameter?

>
>> but have you looked into the
>> container indexing suite for existing __getitem__, __setitem__ and
>> __delitem__ support? Joel's code is currently only in the CVS, but
>> will be included in the 1.31 release of boost, AFAIK. It includes
>> support for Python slices.
>
> Well, I've looked at the indexing suite now, and there are several
> problems with its slice support.
>
> 1) Non-None values of step size are silently ignored.
> I wrote a patch for slice_helper<>::base_get_slice_data() to throw
> IndexError if step size is given.
>
> 2) Does not clip slice ranges to the max/min as appropriate for slices,
> instead it raises IndexError.  (Patch fixes this)
>
> 3) Crashes when given slice indexes that run cross to each other (in STL
> parlance, stop is not reachable from start).  I believe that the
> responsibility for handling this case is in the hands of the respective
> policies classes.  Therefore, my patch targets vector_indexing_suite
> vice indexing_suite for these:
>    foo = bar[-1:0] crashes, should return empty container. (Patched)
>    del bar[-1:0] crashes, should be a no-op. (Patched)
>    bar[-1:0] = foo crashes.  It has weird insert()-like semantics in 
>        practice for lists, but I haven't seen it documented
>        anywhere. (Patch throws IndexError in this case since performing
>        a slice insertion to an empty slice should be an undefined
>        operation).

What I should probably also have said in my original message is that
I'm working on a new version of the indexing suite. It certainly fixes
some of the issues you've identified - have a look for indexing_v2 in
the archives or see http://home.clara.net/raoulgough/boost/. There are
actually a lot of different issues in providing sensible __getitem__
support for C++ containers (for example, take a look at the proxy
support in Joel's suite, or the container_proxy wrapper in mine).

Please not that I'm not trying to put you off contributing code! I
just think it doesn't make sense to duplicate functionality, so you
should probably be aware of the existing work taking place in this
area.

>
> -Jonathan Brandmeyer
>
> ---crash_test.py---
> # Uses the existing vector_indexing_suite_ext.cpp test modul
> from vector_indexing_suite_ext import *
> foo = FloatVec()
> # Weird initialization, why not supported by a constructor?

That's a good question, but it isn't necessarily that easy to
answer. At least, not if you want to use the container's
iterator-based constructor template. e.g. std::vector has a
constructor

template <class InputIterator> 
vector(InputIterator f, InputIterator l, const Allocator& a = Allocator())

which would be the best one to use. I still haven't figured out the
details of providing this.

> foo[:] = [1,2,3,4,5,6]
> def print_vector(foo):
>     s = '[ '
>     for x in foo:
>         s += repr(x) + ' '
>     s += ']'
>     print s

An easier way:

def print_vec(foo):
     print [x for x in foo]

>
> # Should raise IndexError, or print backwards; actually prints the 
> # original
> print_vector(foo[::-1])

That would be because the step value is ignored, right? In any case,
it's very useful to try this kind of test with a real Python list to
see what "should" happen:

>>> v = [1,2,3,4,5]
>>> print v[::-1]
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
TypeError: sequence index must be integer

Would have to look that one up in the Python reference to see if it's
documented! The indexing_v2 suite prints [5,4,3,2] which also can't
really be right.

>
> # Should print the original, but instead raises IndexError
> print_vector(foo[-10:10])

This works in indexing_v2

>
> # Should print an empty vector( "[ ]"); crashes
> print_vector(foo[-1:0])

also works in v2

>
> # Should do nothing; crashes
> del foo[-1:0]

Check.

>
> # I think this should raise IndexError; crashes.
> foo[-1:0] = [7, 8, 9]

With a real python list, it inserts 7, 8, 9 before the last element:

>>> v = [1,2,3,4]
>>> v[-1:0] = [7,8,9]
>>> print v
[1, 2, 3, 7, 8, 9, 4]

This also works the same in v2. What I've done with the indexing_v2
suite is to try and make it perform exactly as a Python list (at
least, when used with a std::vector or similar). Unfortunately, the v2
support won't make it into the next release (1.31) because I'm still
messing around with some changes to it.

-- 
Raoul Gough.
export LESS='-X'