Some serious lint rolling and updating points to have a color attribute

clusterview/colors.py

@@ -0,0 +1,17 @@
+from enum import Enum
+
+
+class Color(str, Enum):
+    BLUE = 'BLUE'
+    BLACK = 'BLACK'
+    GREY = 'GREY'
+
+
+# A simple map from Color -> RGBA 4-Tuple
+# Note: The color values in the tuple are not RGB, but
+#       rather OpenGL percentage values for RGB.
+COLOR_TO_RGBA = {
+    Color.GREY: (0.827, 0.827, 0.826, 0.0),
+    Color.BLUE: (0.118, 0.565, 1.0, 0.0),
+    Color.BLACK: (0.0, 0.0, 0.0, 0.0)
+}

clusterview/mode_handlers.py

@@ -3,15 +3,16 @@ from enum import Enum
 from PyQt5.QtCore import QEvent, Qt
 from PyQt5.QtGui import QCursor
 
+from .colors import Color
 from .exceptions import ExceededWindowBoundsError
 from .mode import Mode
-from .opengl_widget import (get_bb_bottom_right, get_bb_top_left,
-                            set_drawing_event, set_move_bb_top_left,
+from .opengl_widget import (set_drawing_event, set_move_bb_top_left,
                             set_move_bb_bottom_right, reset_move_bbs,
                             viewport_height, viewport_width)
 from .points import PointSet
 from .point_manager import PointManager
 
+
 class __ClickFlag:
 
     # This is the first stage. On mouse release it goes to
@@ -31,6 +32,7 @@ class __ClickFlag:
     # to NONE - we are done.
     SELECTED_MOVED = 3
 
+
 # Size of point for drawing
 __POINT_SIZE = 8
 
@@ -69,6 +71,7 @@ def refresh_point_list(ctx):
 
     ctx.point_list_widget.update()
 
+
 def __handle_add_point(ctx, event):
     """
     Event handler for the add point mode.
@@ -97,8 +100,8 @@ def __handle_add_point(ctx, event):
         PointManager.point_set.clear_selection()
 
         try:
-            # No attribute at the moment.
-            PointManager.point_set.add_point(event.x(), event.y())
+            # No attribute at the moment, default point color is Color.GREY.
+            PointManager.point_set.add_point(event.x(), event.y(), Color.GREY)
         except ExceededWindowBoundsError:
             # The user tried to place a point whos edges would be
             # on the outside of the window. We will just ignore it.
@@ -111,6 +114,7 @@ def __handle_add_point(ctx, event):
         ctx.opengl_widget.update()
         ctx.point_list_widget.update()
 
+
 def __handle_edit_point(ctx, event):
     # TODO: This function and delete definitely need to make sure they are
     #       on a point we have.
@@ -132,6 +136,7 @@ def __handle_edit_point(ctx, event):
     ctx.update()
     # after this remove the point from the list
 
+
 def ogl_keypress_handler(ctx, event):
     """
     A keypress handler attached to the OpenGL widget.
@@ -166,6 +171,7 @@ def ogl_keypress_handler(ctx, event):
 
         ctx.opengl_widget.update()
 
+
 def __handle_move_points(ctx, event):
     """
     A relatively complicated state machine that handles the process of
@@ -239,7 +245,6 @@ def __handle_move_points(ctx, event):
                     if event.y() > __last_mouse_pos[1]:
                         p.move(0, dy)
 
-
                 except ExceededWindowBoundsError:
                     # This point has indicated a move would exceed
                     # it's bounds, so we'll just go to the next
@@ -260,6 +265,7 @@ def __handle_move_points(ctx, event):
 
     ctx.opengl_widget.update()
 
+
 def __handle_delete_point(ctx, event):
 
     __handle_info_updates(ctx, event)
@@ -276,6 +282,7 @@ def __handle_delete_point(ctx, event):
         ctx.opengl_widget.update()
         ctx.point_list_widget.update()
 
+
 def __handle_info_updates(ctx, event):
     """
     Updates data under the "information" header.
@@ -287,7 +294,6 @@ def __handle_info_updates(ctx, event):
         ctx.mouse_position_label.setText(f"{event.x(), event.y()}")
 
 
-
 # Simple dispatcher to make it easy to dispatch the right mode
 # function when the OpenGL window is acted on.
 MODE_HANDLER_MAP = {

clusterview/opengl_widget.py

@@ -19,24 +19,11 @@ from OpenGL.GL import (glBegin, glClearColor, glColor3f, glEnable,
                        glEnd, GL_LINE_LOOP, GL_POINTS,
                        glPointSize, glVertex3f, glViewport)
 
+from .colors import Color, COLOR_TO_RGBA
 from .exceptions import handle_exceptions, InvalidModeError, InvalidStateError
 from .mode import Mode
 from .point_manager import PointManager
 
-class Color(Enum):
-    BLUE = 0
-    BLACK = 1
-    GREY = 2
-
-# A simple map from Color -> RGBA 4-Tuple
-# Note: The color values in the tuple are not RGB, but
-#       rather OpenGL percentage values for RGB.
-COLOR_TO_RGBA = {
-    Color.GREY: (0.827, 0.827, 0.826, 0.0),
-    Color.BLUE: (0.118, 0.565, 1.0, 0.0),
-    Color.BLACK: (0.0, 0.0, 0.0, 0.0)
-}
-
 # Constants set based on the size of the window.
 __BOTTOM_LEFT = (0, 0)
 __WIDTH = None
@@ -167,7 +154,7 @@ def paint_gl():
         # We want to redraw on any change to Mode.OFF so points are preserved -
         # without this, any switch to Mode.OFF will cause a blank screen to
         # render.
-       draw_points(PointManager.point_set, Color.GREY)
+        draw_points(PointManager.point_set)
 
     if (__current_context.mode in [Mode.ADD, Mode.EDIT,
                                    Mode.MOVE, Mode.DELETE] and
@@ -182,7 +169,7 @@ def paint_gl():
             __current_context.mode is Mode.DELETE or
             __current_context.mode is Mode.LOADED):
 
-        draw_points(PointManager.point_set, Color.GREY)
+        draw_points(PointManager.point_set)
 
     elif __current_context.mode is Mode.EDIT:
         raise NotImplementedError("Drawing for EDIT not implemented.")
@@ -191,7 +178,7 @@ def paint_gl():
         # We have to repeatedly draw the points while we are showing the
         # move box.
         if not PointManager.point_set.empty():
-            draw_points(PointManager.point_set, Color.GREY)
+            draw_points(PointManager.point_set)
 
             draw_selection_box(Color.BLACK)
 
@@ -201,7 +188,8 @@ def paint_gl():
             # Mark points that are selected in the bounding box
             # and draw them using the normal function
             highlight_selection()
-            draw_points(PointManager.point_set, Color.GREY)
+            draw_points(PointManager.point_set)
+
 
 def __clamp_x(x):
     """
@@ -214,6 +202,7 @@ def __clamp_x(x):
     x_w = (x / (__WIDTH / 2.0) - 1.0)
     return x_w
 
+
 def __clamp_y(y):
     """
     Y-coordinate clamping function that goes from mouse coordinates to
@@ -225,6 +214,7 @@ def __clamp_y(y):
     y_w = -1.0 * (y / (__HEIGHT / 2.0) - 1.0)
     return y_w
 
+
 def box_hit(tx, ty, x1, y1, x2, y2):
     """
     Calculates whether or not a given point collides with the given bounding
@@ -267,6 +257,7 @@ def box_hit(tx, ty, x1, y1, x2, y2):
             ty >= y1 and
             ty <= y2)
 
+
 def highlight_selection():
     """
     Given the current move bounding box, highlights any points inside it.
@@ -283,6 +274,7 @@ def highlight_selection():
         else:
             point.unselect()
 
+
 def draw_selection_box(color):
     """
     When the move bounding box state is populated and the mode is set
@@ -315,7 +307,6 @@ def draw_selection_box(color):
     # same y as the bottom right.
     bottom_left_corner = (__move_bb_top_left[0], __move_bb_bottom_right[1])
 
-
     glBegin(GL_LINE_LOOP)
     glColor3f(ct[0], ct[1], ct[2])
 
@@ -337,6 +328,7 @@ def draw_selection_box(color):
 
     glEnd()
 
+
 def clear_selection():
     """
     A helper designed to be called from the main window
@@ -347,7 +339,8 @@ def clear_selection():
     if not PointManager.point_set.empty():
         PointManager.point_set.clear_selection()
 
-def draw_points(point_set, color):
+
+def draw_points(point_set):
     """
     Simple point drawing function.
 
@@ -364,10 +357,6 @@ def draw_points(point_set, color):
         raise InvalidStateError("Drawing context must be set before setting " +
                                 "drawing mode")
 
-    if not isinstance(color, Color):
-        raise ValueError("Color must exist in the Color enumeration")
-
-
     glViewport(0, 0, __WIDTH, __HEIGHT)
     glPointSize(PointManager.point_set.point_size)
 
@@ -378,7 +367,7 @@ def draw_points(point_set, color):
             blue = COLOR_TO_RGBA[Color.BLUE]
             glColor3f(blue[0], blue[1], blue[2])
         else:
-            ct = COLOR_TO_RGBA[color]
+            ct = COLOR_TO_RGBA[point.color]
             glColor3f(ct[0], ct[1], ct[2])
 
         glVertex3f(__clamp_x(point.x),

clusterview/point_manager.py

@@ -1,7 +1,9 @@
 import json
 
+from .colors import Color
 from .points import PointSet
 
+
 class PointManager():
     """
     A state class that represents the absolute state of the
@@ -26,7 +28,10 @@ class PointManager():
                                               data['viewport_height'])
 
             for point in data['points']:
-                PointManager.point_set.add_point(point['x'], point['y'])
+                # We will need to cast the string representation of color
+                # back into a Color enum.
+                PointManager.point_set.add_point(point['x'], point['y'],
+                                                 Color(point['color']))
 
     @staticmethod
     def save(location):
@@ -45,7 +50,8 @@ class PointManager():
         for p in PointManager.point_set.points:
             data['points'].append({
                 'x': p.x,
-                'y': p.y
+                'y': p.y,
+                'color': p.color
             })
 
         with open(location, 'w') as out_file:

clusterview/points.py

@@ -1,7 +1,9 @@
 from math import floor
 
+from .colors import Color
 from .exceptions import ExceededWindowBoundsError
 
+
 class Point:
     """
     A class representing a point. A point
@@ -9,23 +11,33 @@ class Point:
     it.
     """
 
-    def __init__(self, x, y, point_size, viewport_width, viewport_height):
+    def __init__(self, x, y, color, point_size,
+                 viewport_width, viewport_height):
         """
-        Initializes a new point with a point_size bounding box.
+        Initializes a new point with a point_size bounding box, viewport
+        awareness, and a color.
 
         Initialized with additional viewport data to make sure the
         move function refuses to move a point outside the screen.
 
         @param x The x-coordinate.
         @param y The y-coordinate.
+        @param color The color of the point.
         @param point_size The size of the point in pixels.
         @param viewport_width The width of the viewport.
         @param viewport_height The height of the viewport.
         """
+
+        if not isinstance(color, Color):
+            raise ValueError("Point must be initialized with a color of " +
+                             "type Color.")
+
         self.__point_size = point_size
         self.__x = x
         self.__y = y
 
+        self.__color = color
+
         self.__viewport_width = viewport_width
         self.__viewport_height = viewport_height
 
@@ -54,6 +66,17 @@ class Point:
         return self.__selected
 
     @property
+    def color(self):
+        return self.__color
+
+    @color.setter
+    def color(self, color):
+        if not isinstance(color, Color):
+            raise ValueError("Point color must be of type Color.")
+
+        self.__color = color
+
+    @property
     def attributes(self):
         return self.__attributes
 
@@ -120,6 +143,8 @@ class Point:
         """
         return (self.__x == other.x and
                 self.__y == other.y and
+                self.__color == other.color and
+                self.__attributes == other.attributes and
                 self.__point_size == other.point_size)
 
     def __repr__(self):
@@ -127,10 +152,11 @@ class Point:
         # For some reason I had to split this instead of using one giant
         # string chained with `+` inside of `()`.
         s = "<POINT "
-        s += "X: {} | Y: {} | ".format(self.__x, self.__y)
-        s += "SIZE: {} | ".format(self.__point_size)
-        s += "VIEWPORT_WIDTH: {} | ".format(self.__viewport_width)
-        s += "VIEWPORT_HEIGHT: {}".format(self.__viewport_height)
+        s += f"X: {self.__x} | Y: {self.__y} | "
+        s += f"SIZE: {self.__point_size} | "
+        s += f"COLOR: {self.__color} | "
+        s += f"VIEWPORT_WIDTH: {self.__viewport_width} | "
+        s += f"VIEWPORT_HEIGHT: {self.__viewport_height}"
         s += ">"
 
         return s
@@ -174,6 +200,7 @@ class Point:
                 y <= self.__top_left_corner[1] and
                 y >= self.__bottom_right_corner[1])
 
+
 class Attribute:
 
     def __init__(self, name, value):
@@ -183,6 +210,7 @@ class Attribute:
         self.__name = name
         self.__value = value
 
+
 class PointSet:
     """
     Useful container for points. Since points are not hashable (they are
@@ -207,36 +235,9 @@ class PointSet:
         self.__viewport_height = viewport_height
 
     def __eq__(self, other):
-        # We are forced to convert other.points from generator to a set to
-        # check equality on the sets. This could possibly get very slow
-        # for large numbers of points.
-
-        attributes_equal = True
-
         other_points = list(other.points)
 
-        attributes_equal = (attributes_equal and
-                           (len(self.__points) == len(other_points)))
-
-        # This is O(N^2) - can it be improved using some sort of
-        # find function?
-        for p in self.__points:
-            for i, op in enumerate(other_points):
-                if p == op:
-                    attributes_equal = (attributes_equal and
-                                        (p.attributes == op.attributes))
-                    continue
-
-                if i == len(other_points) - 1:
-                    # In this case we have enumerated the entire second
-                    # set and not found anything. We can safely say
-                    # the two sets are not equal and return.
-                    attribute_equals = False
-                    break
-
-
         return (self.__points == other_points and
-                attributes_equal and
                 self.__point_size == other.point_size and
                 self.__viewport_width == other.viewport_width and
                 self.__viewport_height == other.viewport_height)
@@ -288,24 +289,28 @@ class PointSet:
         for p in self.__points:
             p.unselect()
 
-    def add_point(self, x, y, attrs=[]):
+    def add_point(self, x, y, color, attrs=[]):
         """
         Adds a point in screen coordinates and an optional attribute to
         the list.
 
         @param x The x-coordinate.
         @param y The y-coordinate.
+        @param color The color of the point.
         @param attr An optional attribute.
         @raises ExceededWindowBoundsError If the point could not be constructed
-                                          because it would be outside the window
-                                          bounds.
+                                          because it would be outside the
+                                          window bounds.
         """
 
         if attrs != [] and not all(isinstance(x, Attribute) for x in attrs):
             raise ValueError("Attributes in add_point must be an " +
                              "attribute array.")
 
-        point = Point(x, y, self.__point_size,
+        if not isinstance(color, Color):
+            raise ValueError("Point color must be a Color enum.")
+
+        point = Point(x, y, color, self.__point_size,
                       self.__viewport_width, self.__viewport_height)
 
         for attr in attrs:

tests/test_point.py

@@ -1,42 +1,49 @@
 import pytest
 
+from clusterview.colors import Color
 from clusterview.exceptions import ExceededWindowBoundsError
 from clusterview.points import Point, PointSet
 
+
 def test_move_point():
     # The minimum starting position is 1/2 point away
     # from the edges
-    p = Point(4, 4, 8, 100, 100)
+    p = Point(4, 4, Color.GREY, 8, 100, 100)
 
     p.move(1, 1)
 
     assert p.x == 5 and p.y == 5
 
+
 def test_attributes_must_be_array_of_attributes():
     with pytest.raises(ValueError):
-        l = PointSet(8, 100, 100)
-        l.add_point(4, 4, attrs=[1,2,3,4,5])
+        point_set = PointSet(8, 100, 100)
+        point_set.add_point(4, 4, Color.GREY, attrs=[1, 2, 3, 4, 5])
+
 
 def test_move_point_outside_screen_x_positive():
-    p = Point(4, 4, 8, 100, 100)
+    pt = Point(4, 4, Color.GREY, 8, 100, 100)
+
+    with pytest.raises(ExceededWindowBoundsError):
+        pt.move(96, 0)
 
-    with pytest.raises(ExceededWindowBoundsError) as exc_info:
-        p.move(96, 0)
 
 def test_move_point_outside_screen_y_positive():
-    p = Point(4, 4, 8, 100, 100)
+    p = Point(4, 4, Color.GREY, 8, 100, 100)
 
     with pytest.raises(ExceededWindowBoundsError):
         p.move(0, 95)
 
+
 def test_move_point_outside_screen_x_negative():
-    p = Point(4, 4, 8, 100, 100)
+    p = Point(4, 4, Color.GREY, 8, 100, 100)
 
     with pytest.raises(ExceededWindowBoundsError):
         p.move(-5, 0)
 
+
 def test_move_point_outside_screen_y_negative():
-    p = Point(4, 4, 8, 100, 100)
+    p = Point(4, 4, Color.GREY, 8, 100, 100)
 
     with pytest.raises(ExceededWindowBoundsError):
         p.move(0, -5)

tests/test_point_manager.py

@@ -3,6 +3,7 @@ import os
 
 import pytest
 
+from clusterview.colors import Color
 from clusterview.points import PointSet
 from clusterview.point_manager import PointManager
 
@@ -10,20 +11,21 @@ from clusterview.point_manager import PointManager
 @pytest.fixture(autouse=True)
 def setup():
     p = PointSet(8, 100, 100)
-    p.add_point(4, 4)
-    p.add_point(9, 10)
-    p.add_point(30, 40)
+    p.add_point(4, 4, Color.GREY)
+    p.add_point(9, 10, Color.GREY)
+    p.add_point(30, 40, Color.GREY)
 
     PointManager.point_set = p
 
+
 def test_load(tmpdir):
     test = ("{\n"
             "\"point_size\": 8,\n"
             "\"viewport_height\": 100,\n"
             "\"viewport_width\": 100,\n"
             "\"points\":[\n"
-            "{\"x\": 8, \"y\": 8},"
-            "{\"x\": 30, \"y\": 50}"
+            "{\"x\": 8, \"y\": 8, \"color\": \"GREY\"},"
+            "{\"x\": 30, \"y\": 50, \"color\": \"GREY\"}"
             "]\n"
             "}")
 
@@ -31,17 +33,16 @@ def test_load(tmpdir):
     p.write(test)
 
     expected = PointSet(8, 100, 100)
-    expected.add_point(8, 8)
-    expected.add_point(30, 50)
+    expected.add_point(8, 8, Color.GREY)
+    expected.add_point(30, 50, Color.GREY)
 
     PointManager.load(p)
 
-    print(PointManager.point_set)
-
     # The fixture point_set inside the singleton PointManager should be
     # overwritten.
     assert PointManager.point_set == expected
 
+
 def test_save(tmpdir):
     d = tmpdir.mkdir("test_data").join("save.json")
 
@@ -52,16 +53,15 @@ def test_save(tmpdir):
                     "\"viewport_height\": 100,"
                     "\"viewport_width\": 100,"
                     "\"points\":["
-                    "{\"x\": 4, \"y\": 4},"
-                    "{\"x\": 9, \"y\": 10},"
-                    "{\"x\": 30, \"y\": 40}"
+                    "{\"x\": 4, \"y\": 4, \"color\": \"GREY\"},"
+                    "{\"x\": 9, \"y\": 10, \"color\": \"GREY\"},"
+                    "{\"x\": 30, \"y\": 40, \"color\": \"GREY\"}"
                     "]"
                     "}")
 
-
     with open(d) as f:
         expected = json.loads(expected_str)
-        actual = json.load(d)
+        actual = json.load(f)
 
         # Since the JSON module converts the `points` key to
         # a list of dicts, we need to do manual comparison

tests/test_point_set.py

@@ -1,89 +1,91 @@
 import pytest
 
+from clusterview.colors import Color
 from clusterview.points import Attribute, Point, PointSet
 
 
 def test_empty():
-    l = PointSet(3, 100, 100)
+    point_set = PointSet(3, 100, 100)
+
+    assert point_set.empty()
 
-    assert l.empty()
 
 def test_add_to_point_set():
-    l = PointSet(3, 100, 100)
+    point_set = PointSet(3, 100, 100)
 
-    l.add_point(1, 2)
+    point_set.add_point(1, 2, Color.GREY)
 
-    points = list(l.points)
+    points = list(point_set.points)
 
-    p = Point(1, 2, 3, 100, 100)
+    p = Point(1, 2, Color.GREY, 3, 100, 100)
     assert len(points) == 1
     assert points[0] == p
 
+
 def test_add_to_point_set_with_attributes():
     attribute = Attribute("thing", 1)
 
-    l = PointSet(3, 100, 100)
-    l.add_point(2, 3, attrs=[attribute])
+    point_set = PointSet(3, 100, 100)
+    point_set.add_point(2, 3, Color.GREY, attrs=[attribute])
 
-    points = list(l.points)
-    point = Point(2, 3, 3, 100, 100)
+    points = list(point_set.points)
 
     assert len(points) == 1
     assert len(points[0].attributes) == 1
 
+
 def test_remove_point_exact_click():
     attribute = Attribute("thing", 1)
 
-    l = PointSet(8, 100, 100)
-    l.add_point(4, 4, attrs=[attribute])
+    point_set = PointSet(8, 100, 100)
+    point_set.add_point(4, 4, Color.GREY, attrs=[attribute])
 
-    p = Point(4, 4, 8, 100, 100)
-    l.remove_point(4, 4)
+    point_set.remove_point(4, 4)
 
-    points = list(l.points)
+    points = list(point_set.points)
 
     assert len(points) == 0
 
+
 def test_remove_point_bounding_box():
     """
     This test checks the bounding box hit heuristic.
     """
     attribute = Attribute("thing", 1)
 
-    l = PointSet(8, 100, 100)
-    l.add_point(4, 4, attrs=[attribute])
-
-    p = Point(4, 4, 8, 100, 100)
+    point_set = PointSet(8, 100, 100)
+    point_set.add_point(4, 4, Color.GREY, attrs=[attribute])
 
     # The click-point (2, 1) will be inside of our point size 8
     # bounding box.
-    l.remove_point(4, 4)
+    point_set.remove_point(4, 4)
 
-    points = list(l.points)
+    points = list(point_set.points)
 
     assert len(points) == 0
 
+
 def test_clear_all_selected_points():
-    l = PointSet(8, 100, 100)
-    l.add_point(4, 4)
-    l.add_point(5, 5)
+    point_set = PointSet(8, 100, 100)
+    point_set.add_point(4, 4, Color.GREY)
+    point_set.add_point(5, 5, Color.GREY)
 
-    for p in l.points:
+    for p in point_set.points:
         p.select()
 
     selected = 0
 
-    for p in l.points:
+    for p in point_set.points:
         if p.selected:
             selected += 1
 
     assert selected == 2
 
-    l.clear_selection()
+    point_set.clear_selection()
 
     unselected = 0
 
-    for p in l.points:
+    for p in point_set.points:
         if not p.selected:
             unselected += 1