patternpythonMinor
Tracking Eye Movements
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:
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:
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:
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):
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_codeAnd 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
Secondly, some comments about the code.
You use
You have made a new class,
The only time you use
...which is equivalent to just
Instead, do
Also, switch the conditional, which becomes:
Erm, do you mean
?
Don't use attributes with two double-underscores (
The funciton definition line should be formatted as
since it's too short to need line wrapping.
Attributes should be in
You never use
You don't need to call
You do (after the move to class
Firstly, this should be touched-up to
Secondly, adding immutable containers in loops is bad. Use
In
I have a feeling
This can be simplified into:
which means that you're returning
In
Your
In
This is just
Your comparison would be better written as
although I suggest you change this to
In fact, returning different types based off of whether
Your name
You only call
Move the error checking in
Don't give
Most of your methods start with
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 = signsYou 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) <= 2Erm, 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 dofor column in self.abacus[from_column:]:
column.current_line = 0do_line_increment should use += forself.abacus[column].current_line += 1I 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 Truewhich 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 doself.do_count(self.base - 1)In
enumerate you do:for state in self.states:
container.append(state)
return containerThis 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 = signstry:
raise BaseError(1)
except BaseError as e:
print 'Use argument "base" higher then one.', e.valueprint '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.