Custom slice() on list subtype

Question

I was working on a subtype of list in python that acts as a list that should never raise an IndexError (every key you input that's greater than the length of the list gets wrapped around). I was having quite a few problems however implementing it properly for slices, who have a LOT of base cases. I wrote the following code to handle slices the way I want to (with example slices commented out);

class Cycle:

    def __getitem__(self, key):
            _self = self._container
            n = len(self)
            if isinstance(key, slice):
                s = key.start
                e = key.stop
                stp = key.step
                r = False
                if stp is None:
                    stp = 1
                elif stp == 0:
                    raise ValueError()
                elif stp < 0:
                    r = True
                    stp = abs(stp)
                if s and not e:
                    sliced = []
                    _len = n - s #[3:]
                    if s < 0:
                        #[-1:]
                        _len = abs(s)
                    elif s >= n:
                        #[7:]
                        _len = n - s % n
                    for i in range(0, _len, stp):
                        index = (i + s) % n
                        sliced.append(_self[index])
                elif e and not s:
                    sliced = []
                    _len = e #[:4]
                    if e <= 0:
                        #[:-1]
                        _len = n - abs(e) % n
                    for i in range(0, _len, stp):
                        index = i % n
                        sliced.append(_self[index])
                elif s and e:
                    sliced = []
                    if s <= e:
                        if s < 0 and e < 0:
                            #[-3:-1]
                            #[-7:-1]
                            _len = abs(s) - abs(e)
                        elif s < 0 and 0 <= e:
                            #[-1:3]
                            #[-7:8]
                            if n + s % n > e:
                                _len = e - s
                            else:
                                _len = e - (n + s)
                        elif 0 <= s and 0 <= e:
                            #[1:3]
                            #[7:19]
                            _len = e - s
                    elif s > e:
                        if s < 0 and e < 0:
                            #[-1:-3]
                            raise ValueError()
                        elif s >= 0 and e < 0:
                            if n + e < s:
                                #[4:-3]
                                raise ValueError()
                            _len = (n + e) - s #[1:-1]
                        elif 0 <= s and 0 <= e:
                            if s >= n:
                                if s % n <= e:
                                    #[7:1]
                                    _len = e - s % n
                                else:
                                    #[7:3]
                                    _len = n - s % n + e
                            else:
                                #[4:2]
                                _len = (n - s % n) + e
                    for i in range(0, _len, stp):
                        index = (i + s) % n
                        sliced.append(_self[index])
                elif not (s or e):
                    sliced = _self[::stp]
                if r:
                    sliced = sliced[::-1]
                return sliced
            else:
                key %= n
                return _self[key]

I can't think of any other more compact way to test for all the different cases that can occur. Can someone help me out please?

Answer 1

I'm not doing a lot of Python but one thing leaping out at me is the usage of all these single letter variables which make the code really hard to read. While longer names will make the code a bit longer I think they will improve readability quite a lot:

• s should be start or rangeStart
• e should be end or rangeEnd
• stp should be step
• r: from just briefly browsing over the code I had no idea what that flag is supposed to represent. Given that it's set to True when the step is negative I assume it could mean reverse.

Answer 2

Please break this code into sub-functions. This one function is 85 lines of hard to follow zig-zagging indentation. I have no idea what the code is doing or why each if clause is significant. Good function (and variable) names will make the code easier to follow without having to know much about the code.

---

Your code is sprinkled with various comments like the following.

_len = n - s #[3:]

This means nothing to me. How does n - s equate to a sub-array starting at the 4th element? Comments should be there to help convey information that the code does not. These comments just confuse me more. If your comment is saying what you are doing, that is an indication that the might be better if it was broken down into smaller pieces. In general, comments should say why you are doing something. Let function names/descriptions tell you what is happening in the code.

Answer 3

From what I can tell, a major cause of complexity of your code is trying to pin down the beginning and/or end of an infinitely long sequence. In my mind, a cyclic list cycles infinitely to both positive and negative directions. It has no beginning or end. Leaving out either start or stop of the slice should be flagged as an error.

Given that, you could just loop for i in range(start,stop,step) and take items at i % len_of_cycle.

---

Trying to figure out what your code does, I added these methods to make your code runnable. (It would be very good to post runnable code to begin with, by the way.)

    def __init__(self, iterable):
        self._container = list(iterable)

    def __len__(self):
        return len(self._container)

I don't quite find the behavior consistent.

Example 1: Here are three ways of selecting the last two items of the cycle. Wouldn't it be logical for the fourth case to do the same?

>>> c = Cycle(range(5))
>>> c[3:]
[3, 4]
>>> c[-2:]
[3, 4]
>>> c[8:]
[3, 4]
>>> c[-7:]
[3, 4, 0, 1, 2, 3, 4]

Example 2: Why does this slice not start from 2?

>>> c[2::-1]
[4, 3, 2]

Example 3: The first two cases treat -1 and 4 the same, like a regular list would. I would expect -4 and 1 behave identically in the latter two cases, even if the behavior differs from list's.

>>> c[1:4]
[1, 2, 3]
>>> c[1:-1]
[1, 2, 3]
>>> c[4:1]
[4, 0]
>>> c[4:-4]
Traceback (most recent call last):
 ...
ValueError

Answer 4

The easiest way to go about this problem is to convert all the input parameters into a standard form.

Lets say the start and end indices will be contained in the following sets:

lowbound -> [0,len) #meaning that the value starts from 0 up to but not including len
highbound -> (0,len] #meaning that the value starts from but not including 0 up to len

and our step variable is in the range

step -> (-inf,0) U (0, inf) #meaning step can be any value but 0

If start or end aren't given, lets set them to there max/min values... Now lets make some formatting functions!!

def FormatLowBound(start, length):
    if start is None:
        return 0
    # if start is less that 0, we need to convert it to a positve number
    # in order to be contained in our defined set for start -> [0,len)
    while start < 0:
        # for example -1 -> len -1 
        # if a number is larger than len it must be subtracted again
        # a while loop will achieve this goal
        start = length + start
    # now imagine start could have be larger than len, so lets mod it:
    start %= length
    # Were done here, return that junk!
    return start

The FormatHighBound function will be very similar to the last, but it has a slightly different range

def FormatHighBound(end, length):
    if end is None:
        return length
    while end < 0:
        end = length + end
    end %= length
    # if end is now the value of zero lets set it to length instaed!
    if end == 0:
        end = length
    return end

Now all we have to do is raise a value error if step is 0, because its the only incorrect value for step- And we also must define our default value as 1

def FormatStep(step):
    if step is None:
        return 1
    if step == 0:
        raise ValueError()
    return step

After this we're left with one final condition that could give us trouble,. If step is negative and and start < end, or step is positive and and start > end; we get an empty array returned...

This however it does not cause an error, so lets leave it as an expected result

Because all the values are now contained in the appropriate range, we can return a slice of Cycle items in that simplified range!

class Cycle(object):
    def __init__(self, val):
        self._container = val
    def __repr__(self):
        return str(self._container)
    def __getitem__(self, key):
            _self = self._container
            length = len(_self)
            if isinstance(key, slice):
                step = FormatStep(key.step)
                # if step is negative there are special none cases to consider
                # this is to include all values for reverse slices with empty params
                if step < 0:
                    if key.stop is None:
                        end = -1
                    else:
                        end = FormatLowBound(key.stop, length)
                    if key.start is None:
                        start = length-1
                    else:
                        start = FormatLowBound(key.start, length)

                else:
                    start = FormatLowBound(key.start, length)
                    end = FormatHighBound(key.stop, length)
                return [_self[index] for index in range(start,end,step)]
            else:
                return _self[key % length]

As you can see, it works nicely!!

>>> c = Cycle(range(10))
>>> c[9::-1]
[9, 8, 7, 6, 5, 4, 3, 2, 1, 0]
>>> c[9:0:-1]
[9, 8, 7, 6, 5, 4, 3, 2, 1]
>>> c[0]
0
>>> c[10]
0
>>> c[9]
9
>>> c[-12341234]
6
>>> c[1:-1:2]
[1, 3, 5, 7]
>>> c[1:-11:2]
[1, 3, 5, 7]
>>> 

Hope this helps!