Successful first drawing and tests around the new PointSet

CONTRIBUTING.md

@@ -25,12 +25,13 @@ This will install all normal requirements as well as testing requirements.
 
 We use `flake8` for linting. 
 
+Run `python -m flake8` in order to get a lint report.
+
 ## Running Tests
 
 We use `pytest` as our testing framework.
 
-
-TODO
+To run tests with test coverage use `pytest --cov=clusterview tests/`.
 
 ## Updating the UI Design File
 

clusterview/exceptions.py

@@ -1,6 +1,30 @@
 from PyQt5.QtWidgets import QErrorMessage
 
-from clusterview.mode import Mode
+from .mode import Mode
+
+
+class InvalidStateError(Exception):
+    pass
+
+
+class InvalidModeError(Exception):
+    """
+    An exception to specify an invalid mode has been provided.
+    """
+
+    def __init__(self, mode):
+        """
+        Initializes the InvalidMode exception with a
+        mode.
+        """
+
+        if not isinstance(mode, Mode):
+            raise ValueError("Mode argument to InvalidMode must be of " +
+                             " type mode")
+
+        # Mode cases for invalid mode
+        if mode == Mode.OFF:
+            super().__init__("You must select a mode before continuing.")
 
 
 def handle_exceptions(func):
@@ -27,25 +51,3 @@ def handle_exceptions(func):
             error_dialog.exec_()
 
     return wrapped
-
-class InvalidStateError(Exception):
-    pass
-
-class InvalidModeError(Exception):
-    """
-    An exception to specify an invalid mode has been provided.
-    """
-
-    def __init__(self, mode):
-        """
-        Initializes the InvalidMode exception with a
-        mode.
-        """
-
-        if not isinstance(mode, Mode):
-            raise ValueError("Mode argument to InvalidMode must be of "+
-                             " type mode")
-
-        # Mode cases for invalid mode
-        if mode == Mode.OFF:
-            super().__init__("You must select a mode before continuing.")

clusterview/mode.py

@@ -1,6 +1,5 @@
 from enum import Enum
 
-from clusterview.opengl_widget import set_drawing_mode
 
 class Mode(Enum):
     """
@@ -13,63 +12,3 @@ class Mode(Enum):
     EDIT = 2
     MOVE = 3
     DELETE = 4
-
-def __handle_add_point(ctx, event):
-    """
-    Event handler for the add point mode.
-
-    Sets the drawing mode for the OpenGL Widget using
-    `set_drawing_mode`, converts a point to our point
-    representation, and adds it to the list.
-    """
-    print("[ADD] GOT POINT: ({}, {})".format(event.x(), event.y()))
-
-    set_drawing_mode(Mode.ADD, event)
-
-    # Convert to our point representation and add to list widget
-    # Point representation is a class called Point with coordinates,
-    # and attributes (currently always None)
-
-def __handle_edit_point(ctx, event):
-    # TODO: This function and delete definitely need to make sure they are
-    #       on a point we have.
-    #
-    # Since points are unique consider a hashmap of points to make O(1)
-    # lookups for addition and deletion. This list can be maintained here
-    # in this module. It should be a dictionary - from point to
-    # attributes in the case of algorithms that require points to have
-    # weights or something.
-    #
-    # Should move the associated point in the list to the new location if
-    # applicable.
-    print("[EDIT] GOT POINT: ({}, {})".format(event.x(), event.y()))
-
-    # Store old x, y from event
-    set_drawing_mode(Mode.DELETE, event)
-    # after this remove the point from the list
-
-def __handle_move_points(ctx, event):
-    # TODO: Should move the associated points in the list to the new location.
-    print("[MOVE] Pressed - NOTE NEED DRAG EVENT")
-    # Store list of old points that are captured
-
-    set_drawing_mode(Mode.MOVE, event)
-
-    # Find and move all points from the old list to their new locations
-
-def __handle_delete_point(ctx, event):
-    print("[DELETE] GOT POINT: ({}, {})".format(event.x(), event.y()))
-
-    set_drawing_mode(Mode.DELETE, event)
-
-    # Find the point from event and remove it from the list
-
-# Simple dispatcher to make it easy to dispatch the right mode
-# function when the OpenGL window is clicked.
-MODE_MAP = {
-    Mode.OFF: lambda: None,
-    Mode.ADD: __handle_add_point,
-    Mode.EDIT: __handle_edit_point,
-    Mode.MOVE: __handle_move_points,
-    Mode.DELETE: __handle_delete_point
-}

clusterview/mode_handlers.py

@@ -0,0 +1,63 @@
+from .mode import Mode
+from .opengl_widget import set_drawing_event
+
+def __handle_add_point(ctx, event):
+    """
+    Event handler for the add point mode.
+
+    Sets the drawing mode for the OpenGL Widget using
+    `set_drawing_mode`, converts a point to our point
+    representation, and adds it to the list.
+    """
+    print("[ADD] GOT POINT: ({}, {})".format(event.x(), event.y()))
+
+    set_drawing_event(event)
+    ctx.update()
+    # Convert to our point representation and add to list widget
+    # Point representation is a class called Point with coordinates,
+    # and attributes (currently always None)
+
+def __handle_edit_point(ctx, event):
+    # TODO: This function and delete definitely need to make sure they are
+    #       on a point we have.
+    #
+    # Since points are unique consider a hashmap of points to make O(1)
+    # lookups for addition and deletion. This list can be maintained here
+    # in this module. It should be a dictionary - from point to
+    # attributes in the case of algorithms that require points to have
+    # weights or something.
+    #
+    # Should move the associated point in the list to the new location if
+    # applicable.
+    print("[EDIT] GOT POINT: ({}, {})".format(event.x(), event.y()))
+
+    # Store old x, y from event
+    set_drawing_event(event)
+    ctx.update()
+    # after this remove the point from the list
+
+def __handle_move_points(ctx, event):
+    # TODO: Should move the associated points in the list to the new location.
+    print("[MOVE] Pressed - NOTE NEED DRAG EVENT")
+    # Store list of old points that are captured
+
+    set_drawing_event(event)
+
+    # Find and move all points from the old list to their new locations
+
+def __handle_delete_point(ctx, event):
+    print("[DELETE] GOT POINT: ({}, {})".format(event.x(), event.y()))
+
+    set_drawing_event(event)
+
+    # Find the point from event and remove it from the list
+
+# Simple dispatcher to make it easy to dispatch the right mode
+# function when the OpenGL window is clicked.
+MODE_HANDLER_MAP = {
+    Mode.OFF: lambda: None,
+    Mode.ADD: __handle_add_point,
+    Mode.EDIT: __handle_edit_point,
+    Mode.MOVE: __handle_move_points,
+    Mode.DELETE: __handle_delete_point
+}

clusterview/opengl_widget.py

@@ -14,23 +14,58 @@ from enum import Enum
 
 from OpenGL.GL import (glBegin, glClearColor, glColor4f, glEnable,
                        glEnd, GL_LIGHT0, GL_LIGHTING, GL_POINTS,
-                       glVertex3f)
+                       glPointSize, glVertex3f, glViewport)
 
-from clusterview.exceptions import InvalidStateError
-from clusterview.mode import InvalidMode, Mode
+from .exceptions import InvalidModeError, InvalidStateError
+from .mode import Mode
 
 class Color(Enum):
     BLUE = 0
 
 # 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.BLUE: (0, 128, 255, 255)
+    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
+__HEIGHT = None
+
+# Module-global state variables for our drawing
+# state machine.
+#
+# Below functions have to mark these as `global` so
+# the interpreter knows that the variables are not
+# function local.
 __current_mode = None
 __current_event = None
+__current_context = None
+
+
+def set_drawing_context(ctx):
+    """
+    Sets the drawing context so that drawing functions can properly
+    interact with the widget.
+    """
+    global __current_context
 
-def set_drawing_mode(mode, event=None):
+    print("CALLING SET DRAWING CONTEXT: {}".format(ctx))
+    __current_context = ctx
+
+    print("WIDTH OF OPENGL WINDOW: {}".format(__WIDTH))
+    print("HEIGHT OF OPENGL WINDOW: {}".format(__HEIGHT))
+
+
+    print("SETTING: {}".format(__current_context))
+
+
+def set_drawing_mode(mode):
     """
     State management function. It is useful to look at the
     different drawing modes as modes in a state machine.
@@ -40,22 +75,44 @@ def set_drawing_mode(mode, event=None):
     on the OpenGL Widget.
 
     @param mode The current mode.
-    @param event The current event (Mostly used for passing coordinates).
     """
+    global __current_context
+    global __current_mode
+
+    print("SET DRAWING MODE CONTEXT: {}".format(__current_context))
+    if __current_context is None:
+        raise InvalidStateError("Drawing context must be set before setting " +
+                                "drawing mode")
+
     if not isinstance(mode, Mode):
         raise ValueError("Mode in set_drawing_mode must be of type Mode")
 
+    print("CALL FROM SET_DRAWING_MODE(MODE = {})".format(mode))
+
     __current_mode = mode
 
-    if event is not None:
-        __current_event = event
+    print("SET LOCALS (CURRENT MODE: {})".format(__current_mode))
 
 
-def get_current_mode():
+def set_drawing_event(event):
     """
-    Returns the current mode according to the OpenGL Widget.
+    State machine event management function.
+
+    @param event The event.
     """
-    return __current_mode
+    global __current_context
+    global __current_event
+
+    if __current_context is None:
+        raise InvalidStateError("Drawing context must be set before setting " +
+                                "drawing mode")
+
+    print("CALL FROM SET_DRAWING_EVENT(event = {})".format(event))
+
+    if event is not None:
+        __current_event = event
+
+    print("SET LOCALS (CURRENT EVENT: {})".format(__current_event))
 
 
 def initialize_gl():
@@ -71,20 +128,36 @@ def initialize_gl():
     glClearColor(255, 255, 255, 0)
 
 
+def resize_gl(w, h):
+    """
+    OpenGL resize handler used to get the current viewport size.
+
+    @param w The new width.
+    @param h The new height.
+    """
+    global __WIDTH
+    global __HEIGHT
+
+    __WIDTH = __current_context.width()
+    __HEIGHT = __current_context.height()
+
+
 def paint_gl():
     """
     Stock PaintGL function from OpenGL that switches
     on the current mode to determine what action to
     perform on the current event.
     """
-
     if (__current_mode in [Mode.ADD, Mode.EDIT, Mode.MOVE, Mode.DELETE] and
         __current_event is None):
         raise InvalidStateError("Event must exist for ADD, EDIT, MOVE, " +
                                 "and DELETE")
 
     if __current_mode is Mode.ADD:
-        raise NotImplementedError("Drawing for ADD not implemented.")
+        # TODO: This needs to be modified to instead take the point list
+        #       and redraw the entire list (which will have the new point
+        #       added) each click.
+        draw_points(__current_event.x(), __current_event.y(), Color.BLUE)
     elif __current_mode is Mode.EDIT:
         raise NotImplementedError("Drawing for EDIT not implemented.")
     elif __current_mode is Mode.MOVE:
@@ -93,7 +166,33 @@ def paint_gl():
         raise NotImplementedError("Drawing for DELETE not implemented.")
 
 
-def draw_point(x, y, color):
+def __clamp_x(x):
+    """
+    X-coordinate clamping function that goes from mouse coordinates to
+    OpenGL coordinates.
+
+    @param x The x-coordinate to clamp.
+    @returns The clamped x coordinate.
+    """
+    x_w = (x / (__WIDTH / 2.0) - 1.0)
+    print(x_w)
+    return x_w
+
+
+def __clamp_y(y):
+    """
+    Y-coordinate clamping function that goes from mouse coordinates to
+    OpenGL coordinates.
+
+    @param y The y-coordinate to clamp.
+    @returns The clamped y coordinate.
+    """
+    y_w = -1.0 * (y / (__HEIGHT / 2.0) - 1.0)
+    print(y_w)
+    return y_w
+
+
+def draw_points(x, y, color):
     """
     Simple point drawing function.
 
@@ -105,15 +204,27 @@ def draw_point(x, y, color):
     @param y The y-coordinate.
     @param color The Color Enum.
     """
+    global __current_context
+
+    if __current_context is None:
+        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")
 
     ct = COLOR_TO_RGBA[color]
 
-    glBegin(GL_POINTS)
+    print("DRAW POINT - DRAWING WITH COLOR {}".format(ct))
+
+    glViewport(0, 0, __WIDTH, __HEIGHT)
+    glPointSize(__POINT_SIZE)
     glColor4f(ct[0], ct[1], ct[2], ct[3])
-    glVertex3f(x, y, 0.0)                 # Z is currently fixed to 0
+
+    glBegin(GL_POINTS)
+    glVertex3f(__clamp_x(__current_event.x()),
+               __clamp_y(__current_event.y()),
+               0.0)                             # Z is currently fixed to 0
     glEnd()
 
 

clusterview/points.py

@@ -0,0 +1,65 @@
+class Attribute:
+    __name = None
+    __value = None
+
+    def __init__(self, name, value):
+        """
+        Initializes an attribute.
+        """
+        self.__name = name
+        self.__value = value
+
+
+class PointSet:
+    """
+    Useful point set for storing coordinates and attributes. It is
+    backed by a set to provide nice convenience functions.
+    """
+    __points = set()
+    __attributes = {}
+
+    def add_point(self, x, y, 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 attr An optional attribute.
+        """
+
+        if attrs != [] and not all(isinstance(x, Attribute) for x in attrs):
+            raise ValueError("Attributes in add_point must be an " +
+                             "attribute array.")
+
+        point = (x, y)
+        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.
+        """
+        point = (x, y)
+        self.__points.discard(point)
+        self.__attributes.pop(point)
+
+
+    def attributes(self, x, y):
+        """
+        Returns the attribute array for a given point.
+
+        @param x The x-coordinate of the point.
+        @param y The y-coordinate of the point.
+        """
+        return self.__attributes[(x, y)]
+
+    @property
+    def points(self):
+        """
+        Getter for points. Returns a generator for
+        looping.
+        """
+        for point in self.__points:
+            yield point

main_window.py

@@ -6,8 +6,10 @@ from PyQt5.QtGui import QCursor
 from PyQt5 import QtWidgets, uic
 
 from clusterview.exceptions import handle_exceptions, InvalidModeError
-from clusterview.mode import Mode, MODE_MAP
-from clusterview.opengl_widget import initialize_gl, paint_gl
+from clusterview.mode import Mode
+from clusterview.mode_handlers import MODE_HANDLER_MAP
+from clusterview.opengl_widget import (initialize_gl, paint_gl, resize_gl,
+                                       set_drawing_mode, set_drawing_context)
 from clusterview_ui import Ui_MainWindow
 
 class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
@@ -23,12 +25,20 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
         super(MainWindow, self).__init__(parent)
         self.setupUi(self)
 
+        # We only need to set the context in our OpenGL state machine
+        # wrapper once here since the window is fixed size.
+        # If we allow resizing of the window, the context must be updated
+        # each resize so that coordinates are converted from screen (x, y)
+        # to OpenGL coordinates properly.
+        set_drawing_context(self.opengl_widget)
+
         #-----------------------------------------------
         # OpenGL Graphics Handlers are set
         # here and defined in clusterview.opengl_widget.
         #-----------------------------------------------
         self.opengl_widget.initializeGL = initialize_gl
         self.opengl_widget.paintGL = paint_gl
+        self.opengl_widget.resizeGL = resize_gl
 
         # -------------------------------------
         # UI Handlers
@@ -49,18 +59,22 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
     #-----------------------------------------------------------------
     def __add_points(self):
         self.__mode = Mode.ADD
+        set_drawing_mode(self.__mode)
         self.opengl_widget.setCursor(QCursor(Qt.CursorShape.CrossCursor))
 
     def __edit_points(self):
         self.__mode = Mode.EDIT
+        set_drawing_mode(self.__mode)
         self.opengl_widget.setCursor(QCursor(Qt.CursorShape.CrossCursor))
 
     def __delete_points(self):
         self.__mode = Mode.DELETE
+        set_drawing_mode(self.__mode)
         self.opengl_widget.setCursor(QCursor(Qt.CursorShape.PointingHandCursor))
 
     def __move_points(self):
         self.__mode = Mode.MOVE
+        set_drawing_mode(self.__mode)
         self.opengl_widget.setCursor(QCursor(Qt.CursorShape.SizeAllCursor))
 
     @handle_exceptions
@@ -86,4 +100,4 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
         # Map from Mode -> function
         # where the function is a handler for the
         # OpenGL Widget.
-        MODE_MAP[self.__mode](self.opengl_widget, event)
+        MODE_HANDLER_MAP[self.__mode](self.opengl_widget, event)

requirements.txt

@@ -1,24 +1,36 @@
+atomicwrites==1.3.0
+attrs==19.1.0
 backcall==0.1.0
+coverage==4.5.4
 Cython==0.29.13
 decorator==4.4.0
 entrypoints==0.3
 flake8==3.7.8
+importlib-metadata==0.19
 ipython==7.7.0
 ipython-genutils==0.2.0
 jedi==0.14.1
 mccabe==0.6.1
+more-itertools==7.2.0
+packaging==19.1
 parso==0.5.1
 pexpect==4.7.0
 pickleshare==0.7.5
+pluggy==0.12.0
 prompt-toolkit==2.0.9
 ptyprocess==0.6.0
+py==1.8.0
 pycodestyle==2.5.0
 pyflakes==2.1.1
 Pygments==2.4.2
 PyOpenGL==3.1.0
 PyOpenGL-accelerate==3.1.3b1
+pyparsing==2.4.2
 PyQt5==5.13.0
 PyQt5-sip==4.19.18
+pytest==5.0.1
+pytest-cov==2.7.1
 six==1.12.0
 traitlets==4.3.2
 wcwidth==0.1.7
+zipp==0.5.2

tests/test_point_set.py

@@ -0,0 +1,52 @@
+import pytest
+
+from clusterview.points import Attribute, PointSet
+
+
+def test_add_to_point_set():
+    l = PointSet()
+
+    l.add_point(1, 2)
+
+    points = list(l.points)
+
+    assert len(points) == 1
+    assert points[0] == (1, 2)
+    assert len(l.attributes(1, 2)) == 0
+
+def test_add_to_point_set_with_attributes():
+    attribute = Attribute("thing", 1)
+
+    l = PointSet()
+    l.add_point(1, 2, attrs=[attribute])
+
+    points = list(l.points)
+    attrs = l.attributes(1, 2)
+
+    assert len(points) == 1
+    assert points[0] == (1, 2)
+    assert len(l.attributes(1, 2)) == 1
+
+def test_remove_point():
+    attribute = Attribute("thing", 1)
+
+    l = PointSet()
+    l.add_point(1, 2, attrs=[attribute])
+
+    l.remove_point(1, 2)
+
+    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(1, 2)
+
+def test_attributes_must_be_array_of_attributes():
+    with pytest.raises(ValueError):
+        l = PointSet()
+        l.add_point(1, 2, attrs=[1,2,3,4,5])