HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Tracking Eye Movements

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
trackingmovementseye

Problem

The following class (that was my first class ever in python) was used in an eye tracking context. Imagine a context where you always will have a single gaze-point and some closed contours. How can I return all possible relations between them?

For example, for two contours, the point can be:

  • Relation 1: inside contour A and outside contour B;



  • Relation 2: inside contour B and outside contour A;



  • Relation 3: inside contours A and B;



  • Relation 4: outside contours A and B;



In my case, I am using each relation as tags for colors in a context where the number of contours changes.

You can imagine the contours as sets as well, but let's avoid the complexities of an advanced Set Theory.

This problem give us 2 to the power c = number of contours. An Abacus give us the number of columns to the power i = number of lines. So, an abacus seems to be good metaphor, for example:

contours = [c1, c2]
Abacus = ClassAbacus (len(contours))
color_tags = Abacus.Enumerate()


give us a list with all possible tags where + is inside, - is outside and numbers are contours:


['+1+2', '+1-2', '-1+2', '-1-2']

So, now we have reference for 4 colors. We could give a name for each point as such:

import cv2

...

def PolygonTestEx(contours, pt, contours_counter = 0, counter_code = ''):
    for x in xrange(1, len(contours) + 1):
        Inside = cv2.pointPolygonTest(contours[x -1], pt, False)
        # Inside = contours[x]
        if Inside > -1:
            counter_code = counter_code + '+' + str(x + contours_counter)
        else:
            counter_code = counter_code + '-' + str(x + contours_counter)
    contours_counter = contours_counter + len(contours)
    return contours_counter, counter_code


And it should give us the 4 possible states in this context:

Could I write this class with better readability?

Please, fell free to suggest a faster way to do the same or missed common practices in the OOP.

```
CURRENT_LINE = 0
SIGNS = 1

class BaseError(Exception):

Solution

I don't have cv2 installed (yet) so I'm going blind here, but I have a few comments. First, the problem you described can be solved very easily with itertools.product:

import itertools

def get_set_intersections(chars="+-", base=2):
    numbers = range(1, base+1)

    for signs in itertools.product(chars, repeat=base):
        yield "".join("{}{}".format(sign, n) for sign, n in zip(signs, numbers))

list(get_set_intersections())
#>>> ['+1+2', '+1-2', '-1+2', '-1-2']


Secondly, some comments about the code.

You use CURRENT_LINE and SIGNS as constant indexes into a list of length 2. You should use a class instead. Here's one:

class AbacusColumn(object):
    def __init__(self, current_line, signs):
        self.current_line = current_line
        self.signs = signs


You have made a new class, BaseError. You shouldn't create new errors where you old ones suffice, though; just use ValueError. Even if you do want a new class, it should be a subclass of ValueError.

The only time you use BaseError you do:

try:
    raise BaseError(1)
except BaseError as e:
    print 'Use argument "base" higher then one.', e.value


...which is equivalent to just

print 'Use argument "base" higher then one. 1'


Instead, do

raise ValueError('Use argument "base" higher then one.')


Also, switch the conditional, which becomes:

if (base + 1) <= 2


Erm, do you mean

if base <= 1


?

Don't use attributes with two double-underscores (self.__Base). Use a single underscore. Two gives you name mangling, which you don't want.

The funciton definition line should be formatted as

def __init__(self, char_set=['+', '-'], base=2):


since it's too short to need line wrapping.

Attributes should be in snake_case.

You never use self.Base. Remove it. Actually, remove _Base instead since you can just add one at point of use.

You don't need to call super; your superclass is object.

You do (after the move to class AbacusColumn)

state = ''
for column in xrange(0, len(self.abacus)):
    state = state + self.abacus[column].signs[self.abacus[column].current_line]


Firstly, this should be touched-up to

state = ''
for column in self.abacus:
    state += column.signs[column.current_line]


Secondly, adding immutable containers in loops is bad. Use str.join instead:

state = ''.join(column.signs[column.current_line] for column in self.abacus)


In do_line_reset you should similarly do

for column in self.abacus[from_column:]:
    column.current_line = 0


do_line_increment should use += for

self.abacus[column].current_line += 1


I have a feeling keeps_counting is broken in do_count. The logic is:

keeps_counting = True

if ...:
    keeps_counting = False
else:
    if ...:
        keeps_counting = self.do_count(column)
        return keeps_counting

    if ...:
        return keeps_counting

    if ...:
        return keeps_counting

[function ends]


This can be simplified into:

if ...:
    ...
else:
    if ...:
        return self.do_count(column)
    if ...:
        return True
    if ...:
        return True


which means that you're returning None instead of False in the first branch or if all three ifs fail. However, it's OK as you never use the result. Remove it.

In instantiate, you can simplify generating self.abacus with a list comprehension:

self.abacus = [
    AbacusColumn(0, [char + str(x+1) for char in self.chars])
    for x in range(self.base)
]


Your max(self.abacus) will have to change since AbacusColumn is not comparable, but it was flawed anyway since the were all initialized as [0, list of strings]. Since the strings are all of the same form bar the numbers you added to them, you're actually just finding the largest index (although since you're doing string comparisons it breaks for bases larger than 9). just do

self.do_count(self.base - 1)


In enumerate you do:

for state in self.states:
    container.append(state)
return container


This is just

return list(self.states)


Your comparison would be better written as

if not (0 < index < self.counter):


although I suggest you change this to

if not (0 <= index < self.counter):


In fact, returning different types based off of whether index is out of bounds is silly and you should just return list(self.states) all the time. If you want a second way to access only one, make a new method.

Your name ClassAbacus is odd: surely just Abacus is better.

You only call instantiate from __init__, so I'd suggest moving it there. If not in the function, then below the function. Doing so allows you to remove self.base and self.chars.

Move the error checking in __init__ to the top.

Don't give self.abacus a default; it only hides bugs.

Most of your methods start with do_. Personally, I would remove it. Spend the characters on better names (eg. what does

Code Snippets

import itertools

def get_set_intersections(chars="+-", base=2):
    numbers = range(1, base+1)

    for signs in itertools.product(chars, repeat=base):
        yield "".join("{}{}".format(sign, n) for sign, n in zip(signs, numbers))

list(get_set_intersections())
#>>> ['+1+2', '+1-2', '-1+2', '-1-2']
class AbacusColumn(object):
    def __init__(self, current_line, signs):
        self.current_line = current_line
        self.signs = signs
try:
    raise BaseError(1)
except BaseError as e:
    print 'Use argument "base" higher then one.', e.value
print 'Use argument "base" higher then one. 1'
raise ValueError('Use argument "base" higher then one.')

Context

StackExchange Code Review Q#75524, answer score: 7

Revisions (0)

No revisions yet.