You shouldn't inherit from builtin data types - there's a UserList type that's more appropriate.
On top of that, you probably shouldn't inherit from UserList either: your code is not a list and doesn't fill its contractual obligations. This is a time to favour composition over inheritance.
Before I look at implementing this, here are a few quick things I noticed. One is
if isinstance(item, tuple):
...
in __getitem__, which doesn't seem to work since _adj_index is packing any tuples it gets into another tuple.
Another thing was the use of for i in xrange(len(self)):, which would be much simpler if you could just do for i in self._items - another advantage of composition. You can actually manage this with super(Ring, self).__iter__(), though.
Your __getslice__ returns another ring; this seems strange - a slice of a ring is rarely itself also a ring. If you have a ring of paper and cut a section out, that section has ends!
Finally, your __str__ and __repr__ are the default ones for list, which is entirely misleading and very bad practice.
Back to the point, looking at the abstract base classes in collections (later moved to collections.abc), the only interfaces it makes sense to support are Container and Iterable, since the class doesn't have a __len__. Adding __{get,set,del}item__ and __repr__ on top of that would give something like
def _adj_index(method):
def _dec(instance, *args):
if isinstance(args[0], int):
index = args[0] % len(instance._items)
return method(instance._items, index, *args[1:])
return method(instance._items, args)
return _dec
class Ring:
"""
Ring datatype.
"""
def __init__(self, items):
self._items = list(items)
def __iter__(self):
while True:
for i in xrange(len(self._items)):
yield self._items[i]
__getitem__ = _adj_index(list.__getitem__)
__setitem__ = _adj_index(list.__setitem__)
__delitem__ = _adj_index(list.__delitem__)
def __getslice__(self, i, j):
return [self[x] for x in range(i, j)]
def __repr__(self):
return "Ring({})".format(self._items)
This discards __getitem__ since it's no longer needed but keeps it similar to your code.
__iter__ could just delegate to itertools.cycle.
__getslice__ is deprecated and has been for ages. Let's consider doing this properly:
def __getitem__(self, idx):
if isinstance(idx, slice):
return ???
return self._items[idx % len(self._items)]
What should ??? be? Well, we'd like something akin to
[self[i] for i in xrange(idx.start, idx.stop, idx.step)]
but start, stop or step might be None. We can say that start and stop should not be None (since both directions extend infinitely), but step can reasonably be asked to default to 1. Unfortunately, slice.indices isn't appropriate here, as with other tricks you can normally use.
Thus:
step = 1 if idx.step is None else idx.step
return [self[i] for i in xrange(idx.start, idx.stop, step)]
Now, __setitem__ probably has no good semantics for slice assignment because:
If a slice is larger than len(self._items), there is no obvious cannonical mapping on how to handle the assignment.
If a slice crosses through the start index (has idx % len(self._items) == 0) and assigns to a slice of a different size, there is no obvious way to decide where the new start is moved to.
You could restrict slice assignments to slices of the same size, and on slices no more than len(self._items) in size - but then you're not giving much bang for the buck. For this reason, I think __setitem__ and __delitem__ should remain as they are, operating only on integers.
However, _adj_index is not worth its cost. You could just implement them as
def __setitem__(self, idx, value):
if not isinstance(idx, int):
raise TypeError("Ring indices must be integers, not {}".format(type(idx)))
self._items[idx % len(self._items)] = value
def __delitem__(self, idx):
if not isinstance(idx, int):
raise TypeError("Ring indices must be integers, not {}".format(type(idx)))
del self._items[idx % len(self._items)]
(the error mirrors that of indexing lists).
In the scheme of things, this code is simpler, faster and contains not too much logical duplication. However, deduplication is nice, so one could merely add a _from_integer_index method:
def _from_integer_index(self, idx):
if not isinstance(idx, int):
raise TypeError("Ring indices must be integers, not {}".format(type(idx)))
return idx % len(self._items)
def __getitem__(self, idx):
if isinstance(idx, slice):
step = 1 if idx.step is None else idx.step
return [self[i] for i in xrange(idx.start, idx.stop, step)]
return self._items[self._from_integer_index(idx)]
def __setitem__(self, idx, value):
self._items[self._from_integer_index(idx)] = value
def __delitem__(self, idx):
del self._items[self._from_integer_index(idx)]
In this case, wrapper methods do not seem to be the most appropriate solution to the problem.
This also allows checking for indexing 0-length rings.
All together:
import itertools
class Ring:
"""
Ring datatype.
"""
def __init__(self, items):
self._items = list(items)
def __iter__(self):
return itertools.cycle(self._items)
def _from_integer_index(self, idx):
if not isinstance(idx, int):
raise TypeError("Ring indices must be integers, not {}".format(type(idx)))
if not len(self._items):
raise IndexError("Indexing empty ring")
return idx % len(self._items)
def __getitem__(self, idx):
if isinstance(idx, slice):
step = 1 if idx.step is None else idx.step
return [self[i] for i in xrange(idx.start, idx.stop, step)]
return self._items[self._from_integer_index(idx)]
def __setitem__(self, idx, value):
self._items[self._from_integer_index(idx)] = value
def __delitem__(self, idx):
del self._items[self._from_integer_index(idx)]
def __repr__(self):
return "Ring({})".format(self._items)
