From e046cf26a4f9fd8d1939f7792e4103dfe1e840a4 Mon Sep 17 00:00:00 2001 From: Taylor Bockman Date: Sat, 10 Aug 2019 01:07:01 -0700 Subject: [PATCH] Successful first drawing and tests around the new PointSet --- CONTRIBUTING.md | 5 +- clusterview/exceptions.py | 48 ++++++++------- clusterview/mode.py | 61 ------------------ clusterview/mode_handlers.py | 63 +++++++++++++++++++ clusterview/opengl_widget.py | 143 ++++++++++++++++++++++++++++++++++++++----- clusterview/points.py | 65 ++++++++++++++++++++ conftest.py | 0 main_window.py | 20 +++++- requirements.txt | 12 ++++ tests/__init__.py | 0 tests/test_point_set.py | 52 ++++++++++++++++ 11 files changed, 364 insertions(+), 105 deletions(-) create mode 100644 clusterview/mode_handlers.py create mode 100644 clusterview/points.py create mode 100644 conftest.py create mode 100644 tests/__init__.py create mode 100644 tests/test_point_set.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 519e2b6..5f00d6c 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/clusterview/exceptions.py b/clusterview/exceptions.py index 67fcbd9..3b66d60 100644 --- a/clusterview/exceptions.py +++ b/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.") diff --git a/clusterview/mode.py b/clusterview/mode.py index 5b361b9..397484d 100644 --- a/clusterview/mode.py +++ b/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 -} diff --git a/clusterview/mode_handlers.py b/clusterview/mode_handlers.py new file mode 100644 index 0000000..dc0da94 --- /dev/null +++ b/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 +} diff --git a/clusterview/opengl_widget.py b/clusterview/opengl_widget.py index 9e02be6..0d9abb8 100644 --- a/clusterview/opengl_widget.py +++ b/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() diff --git a/clusterview/points.py b/clusterview/points.py new file mode 100644 index 0000000..1850ca4 --- /dev/null +++ b/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 diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..e69de29 diff --git a/main_window.py b/main_window.py index 6710602..4f5b347 100644 --- a/main_window.py +++ b/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) diff --git a/requirements.txt b/requirements.txt index d3239ce..7c8348d 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_point_set.py b/tests/test_point_set.py new file mode 100644 index 0000000..ace40f7 --- /dev/null +++ b/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])