Update point set to function off of bounding boxes and add delete feature

clusterview.py

@@ -9,7 +9,6 @@ def main():
     app = QApplication(sys.argv)
 
     window = MainWindow()
-
     window.show()
 
     sys.exit(app.exec_())

clusterview/mode_handlers.py

@@ -2,9 +2,26 @@ from .mode import Mode
 from .opengl_widget import set_current_points, set_drawing_event
 from .points import PointSet
 
+# Size of point for drawing
+__POINT_SIZE = 8
+
 # There are a lot of module-global variables being used because of the
 # nature of state management in OpenGL.
-__point_set = PointSet()
+__point_set = PointSet(__POINT_SIZE)
+
+def __refresh_point_list(ctx):
+    """
+    Refreshes the point list display.
+
+    @param ctx A handle to the window context.
+    """
+    # In order to make some guarantees and avoid duplicate
+    # data we will clear the point list widget and re-populate
+    # it using the current __point_set.
+    ctx.point_list_widget.clear()
+
+    for p in __point_set.points:
+        ctx.point_list_widget.addItem("({}, {})".format(p.x, p.y))
 
 
 def __handle_add_point(ctx, event):
@@ -23,17 +40,11 @@ def __handle_add_point(ctx, event):
     # No attribute at the moment.
     __point_set.add_point(event.x(), event.y())
 
+    __refresh_point_list(ctx)
+
     set_drawing_event(event)
     set_current_points(__point_set)
 
-    # In order to make some guarantees and avoid duplicate
-    # data we will clear the point list widget and re-populate
-    # it using the current __point_set.
-    ctx.point_list_widget.clear()
-
-    for p in __point_set.points:
-        ctx.point_list_widget.addItem("({}, {})".format(p[0], p[1]))
-
     ctx.opengl_widget.update()
     ctx.point_list_widget.update()
 
@@ -66,8 +77,13 @@ def __handle_move_points(ctx, event):
 
 def __handle_delete_point(ctx, event):
     set_drawing_event(event)
-    ctx.update()
-    # Find the point from event and remove it from the list
+
+    __point_set.remove_point(event.x(), event.y())
+
+    __refresh_point_list(ctx)
+
+    ctx.opengl_widget.update()
+    ctx.point_list_widget.update()
 
 # Simple dispatcher to make it easy to dispatch the right mode
 # function when the OpenGL window is clicked.

clusterview/opengl_widget.py

@@ -30,9 +30,6 @@ COLOR_TO_RGBA = {
     Color.BLUE: (0, 0.5, 1.0, 0.0)
 }
 
-# Size of point for drawing
-__POINT_SIZE = 8
-
 # Constants set based on the size of the window.
 __BOTTOM_LEFT = (0, 0)
 __WIDTH = None
@@ -163,7 +160,12 @@ def paint_gl():
         raise InvalidStateError("Points must exist for ADD, EDIT, MOVE, " +
                                 "and DELETE")
 
-    if __current_mode is Mode.ADD:
+    if __current_mode is Mode.ADD or __current_mode is Mode.DELETE:
+        # Note that drawing the points doesn't require a bounding box or
+        # any special context, so delete just removes the element from
+        # the point set, which will be redrawn here. This action
+        # is the same as adding a point since we just draw what is in
+        # the point set.
         draw_points(__current_points, Color.BLUE)
     elif __current_mode is Mode.EDIT:
         raise NotImplementedError("Drawing for EDIT not implemented.")
@@ -220,25 +222,15 @@ def draw_points(point_set, color):
     ct = COLOR_TO_RGBA[color]
 
     glViewport(0, 0, __WIDTH, __HEIGHT)
-    glPointSize(__POINT_SIZE)
-    glColor4f(ct[0], ct[1], ct[2], ct[3])
+    glPointSize(__current_points.point_size)
+
 
     glBegin(GL_POINTS)
+    glColor4f(ct[0], ct[1], ct[2], ct[3])
     for point in point_set.points:
-        glVertex3f(__clamp_x(point[0]),
-                   __clamp_y(point[1]),
+        glVertex3f(__clamp_x(point.x),
+                   __clamp_y(point.y),
                    0.0)                 # Z is currently fixed to 0
     glEnd()
 
 
-def delete_point(x, y):
-    """
-    Deletes a point.
-
-    The list deletion happens in the clusterview.mode module. This
-    function just overwrites the point color with the background.
-
-    @param x The x-coordinate.
-    @param y The y-coordinate.
-    """
-    raise NotImplementedError("delete_point not implemented.")

clusterview/points.py

@@ -1,6 +1,101 @@
+from math import floor
+
+class Point:
+    """
+    A class representing a point. A point
+    has a point_size bounding box around
+    it.
+    """
+
+    def __init__(self, x, y, point_size):
+        """
+        Initializes a new point with a point_size bounding box.
+
+        @param point_size The size of the point in pixels.
+        @param x The x-coordinate.
+        @param y The y-coordinate.
+        """
+        self.__point_size = point_size
+        self.__x = x
+        self.__y = y
+
+        half_point = floor(point_size / 2)
+
+        self.__top_left_corner = (self.__x - half_point,
+                                  self.__y + half_point)
+
+        self.__bottom_right_corner = (self.__x + half_point,
+                                      self.__y - half_point)
+
+    @property
+    def x(self):
+        return self.__x
+
+
+    @property
+    def y(self):
+        return self.__y
+
+
+    @property
+    def point_size(self):
+        return self.__point_size
+
+
+    def __eq__(self, other):
+        """
+        Override for class equality.
+
+        @param other The other object.
+        """
+        return (self.__x == other.x and
+                self.__y == other.y and
+                self.__point_size == other.point_size)
+
+
+    def __hash__(self):
+        """
+        Overridden hashing function so it can be used as a dictionary key
+        for attributes.
+        """
+        return hash((self.__x, self.__y, self.__point_size))
+
+
+    def __repr__(self):
+        return "POINT<X :{} Y: {} SIZE: {}>".format(self.__x,
+                                                    self.__y,
+                                                    self.__point_size)
+
+
+    def hit(self, x, y):
+        """
+        Determines if the point was hit inside of it's bounding box.
+
+        The condition for hit is simple - consider the following
+        bounding box:
+                 -------------
+                |              |
+                |    (x,y)     |
+                |              |
+                 -------------
+
+        Where the clicked location is in the center. Then the top
+        left corner is defined as (x - half_point_size, y + half_point_size)
+        and the bottom corner is (x + half_point_size, y - half_point_size)
+
+        So long as x and y are greater than the top left and less than the
+        top right it is considered a hit.
+
+        This function is necessary for properly deleting and selecting points.
+        """
+
+        return (x >= self.__top_left_corner[0] and
+                x <= self.__bottom_right_corner[0] and
+                y <= self.__top_left_corner[1] and
+                y >= self.__bottom_right_corner[1])
+
+
 class Attribute:
-    __name = None
-    __value = None
 
     def __init__(self, name, value):
         """
@@ -12,11 +107,35 @@ class Attribute:
 
 class PointSet:
     """
-    Useful point set for storing coordinates and attributes. It is
-    backed by a set to provide nice convenience functions.
+    Useful container for points backed by a set to insure point
+    uniqueness.
+    """
+
+    def __init__(self, point_size):
+        """
+        Initializes a point container with points of size point_size.
+
+        @param point_size The size of the points.
+        """
+        self.__points = set()
+        self.__attributes = {}
+        self.__point_size = point_size
+
+
+    @property
+    def points(self):
+        """
+        Getter for points. Returns a generator for
+        looping.
         """
-    __points = set()
-    __attributes = {}
+        for point in self.__points:
+            yield point
+
+
+    @property
+    def point_size(self):
+        return self.__point_size
+
 
     def add_point(self, x, y, attrs=[]):
         """
@@ -32,34 +151,43 @@ class PointSet:
             raise ValueError("Attributes in add_point must be an " +
                              "attribute array.")
 
-        point = (x, y)
+        point = Point(x, y, self.__point_size)
         self.__points.add(point)
         self.__attributes[point] = attrs
 
 
     def remove_point(self, x, y):
         """
-        Removes a point and it's attributes from the point set.
+        Removes a point and it's attributes from the point set based
+        on a bounding box calculation.
+
+        Removing a point is an exercise is determining which points
+        have been hit, and then pulling them out of the list.
+
+        If two points have a section overlapping, and the user clicks
+        the overlapped section, both points will be removed.
+
+        Currently O(n).
+
+        @param x The x-coordinate.
+        @param y The y-coordinate.
         """
-        point = (x, y)
-        self.__points.discard(point)
+
+        # Find points that match
+        matched = set(p for p in self.__points if p.hit(x, y))
+
+        # In place set difference
+        self.__points = self.__points - matched
+
+        # Remove associated attributes
+        for point in matched:
             self.__attributes.pop(point)
 
 
-    def attributes(self, x, y):
+    def attributes(self, point):
         """
         Returns the attribute array for a given point.
 
-        @param x The x-coordinate of the point.
-        @param y The y-coordinate of the point.
+        @param point The point to get attributes for..
         """
-        return self.__attributes[(x, y)]
-
-    @property
-    def points(self):
-        """
-        Getter for points. Returns a generator for
-        looping.
-        """
-        for point in self.__points:
-            yield point
+        return self.__attributes[point]

tests/test_point_set.py

@@ -1,38 +1,43 @@
 import pytest
 
-from clusterview.points import Attribute, PointSet
+from clusterview.points import Attribute, Point, PointSet
 
 
 def test_add_to_point_set():
-    l = PointSet()
+    l = PointSet(3)
 
     l.add_point(1, 2)
 
     points = list(l.points)
 
+    p = Point(1, 2, 3)
     assert len(points) == 1
-    assert points[0] == (1, 2)
-    assert len(l.attributes(1, 2)) == 0
+    assert points[0] == p
+    assert len(l.attributes(p)) == 0
+
 
 def test_add_to_point_set_with_attributes():
     attribute = Attribute("thing", 1)
 
-    l = PointSet()
-    l.add_point(1, 2, attrs=[attribute])
+    l = PointSet(3)
+    l.add_point(2, 3, attrs=[attribute])
 
     points = list(l.points)
-    attrs = l.attributes(1, 2)
+    point = Point(2, 3, 3)
+    attrs = l.attributes(point)
 
     assert len(points) == 1
-    assert points[0] == (1, 2)
-    assert len(l.attributes(1, 2)) == 1
+    assert points[0] == point
+    assert len(l.attributes(point)) == 1
+
 
-def test_remove_point():
+def test_remove_point_exact_click():
     attribute = Attribute("thing", 1)
 
-    l = PointSet()
+    l = PointSet(8)
     l.add_point(1, 2, attrs=[attribute])
 
+    p = Point(1, 2, 8)
     l.remove_point(1, 2)
 
     points = list(l.points)
@@ -44,9 +49,37 @@ def test_remove_point():
         # point to raise a KeyError because it no
         # longer exists in the point -> attribute_list
         # dictionary.
-        l.attributes(1, 2)
+        l.attributes(p)
+
+
+def test_remove_point_bounding_box():
+    """
+    This test checks the bounding box hit heuristic.
+    """
+    attribute = Attribute("thing", 1)
+
+    l = PointSet(8)
+    l.add_point(1, 2, attrs=[attribute])
+
+    p = Point(1, 2, 8)
+
+    # The click-point (2, 1) will be inside of our point size 8
+    # bounding box.
+    l.remove_point(2, 1)
+
+    points = list(l.points)
+
+    assert len(points) == 0
+
+    with pytest.raises(KeyError):
+        # We expect a call to attributes on a removed
+        # point to raise a KeyError because it no
+        # longer exists in the point -> attribute_list
+        # dictionary.
+        l.attributes(p)
+
 
 def test_attributes_must_be_array_of_attributes():
     with pytest.raises(ValueError):
-        l = PointSet()
+        l = PointSet(8)
         l.add_point(1, 2, attrs=[1,2,3,4,5])