ColorWheel and ColorPicker: simplify, cleans up and add tests (#8492

)

ColorWheel/ColorPicker had a number of issues addressed:

* Major changes:
  * `ColorWheel._hsv` was unused (and was an "attractive nuisance" for developers - see #6585).
   * Removed.
 * `ColorWheel` had attributes not defined in `__init__()`
   * Defined them.
 * `init_wheel()` had unused parameter (perhaps it was intended to be scheduled as a Clock event, but never was?)
   * Removed parameter.
 * `init_wheel()` was public, but doesn't need to be called by clients (and was an "attractive nuisance" for developers - see #6585).
   * Made private by adding underscore prefix.
 * `_init_wheel()` was misleading about its role, as it is may be called often.
   * Renamed to `_reset_canvas()`
 * Add basic unit-tests for Color Picker and Color Wheel.
   * Before,  `colorpicker.py` had 0% coverage! This is a long, long way from a complete set of unit-tests of each of the features of these Widgets. It now has 60% statement coverage, which is way too low, but 20% above the project average. It is enough to catch #6585 regressing.
 * ColorWheel: only Red had a stated default value in the documentation. This is important, because ColorPicker and ColorWheel differ over default values.
   * Added default values to default values to documentation

I considered whether to add back `init_wheel(dt)`, mark it deprecated and have it do nothing but `pass`, but the whole API is marked experimental,
so I didn't feel the need.

* Trivial changes due to IDE style-checks complaining
 * Unnecessary parenthesis around tuples
   * Removed.
 * comparison expression could be simplified
  *  Simplified
 * Type-check was complaining about `max(0, float(text))`, which seems perfectly fine to me.
  * Changed it to max(0.0, float(text)), which shut up the type-checker, but also removed the need for several more calls to float().
 * Import order wasn't pep8 compliant
    * Sorted imports

kivy/tests/test_uix_colorpicker.py

@@ -0,0 +1,77 @@
+"""
+Color Wheel and Color Picker Tests
+==================================
+"""
+
+from kivy.tests.common import GraphicUnitTest, UTMotionEvent
+from kivy.uix.colorpicker import ColorWheel, ColorPicker
+
+
+class ColorWheelTest(GraphicUnitTest):
+    def test_render(self):
+        color_wheel = ColorWheel()
+        self.render(color_wheel)
+
+    def test_clicks(self):
+        color_wheel = ColorWheel()
+
+        # ColorPicker has a stated default colour (opaque white).
+        # ColorWheel has a different default Color (transparent black).
+        self.assertEqual(color_wheel.color, [0, 0, 0, 0])
+
+        # Click on corner of widget
+        pos = (color_wheel.pos[0], color_wheel.pos[1])
+        touch = UTMotionEvent(
+            "unittest",
+            1,
+            {
+                "x": pos[0],
+                "y": pos[1],
+            },
+        )
+        touch.grab_current = color_wheel
+        touch.pos = pos
+
+        color_wheel.on_touch_down(touch)
+        color_wheel.on_touch_up(touch)
+
+        # Too far from the center. No effect.
+        self.assertEqual(color_wheel.color, [0, 0, 0, 0])
+
+        pos = (
+            color_wheel.pos[0] + color_wheel.size[0] / 2,
+            color_wheel.pos[1] + color_wheel.size[1] / 4,
+        )
+        # Click in middle, half-the-radius up.
+        touch = UTMotionEvent(
+            "unittest",
+            1,
+            {"x": pos[0], "y": pos[1]},
+        )
+        touch.grab_current = color_wheel
+        touch.pos = pos
+
+        color_wheel.on_touch_down(touch)
+        color_wheel.on_touch_up(touch)
+
+        self.assertEqual(color_wheel.color, [0.75, 0.5, 1, 1])
+
+
+class ColorPickerTest(GraphicUnitTest):
+    def test_render(self):
+        color_picker = ColorPicker()
+        self.render(color_picker)
+
+    def test_set_colour(self):
+        color_picker = ColorPicker()
+        # ColorPicker has a stated default colour (opaque white).
+        # ColorWheel has a different default Color (transparent black).
+        self.assertEqual(color_picker.color, [1, 1, 1, 1])
+
+        # Set without alpha
+        color_picker.set_color((0.5, 0.6, 0.7))
+        self.assertEqual(color_picker.color, [0.5, 0.6, 0.7, 1])
+
+        # Set with alpha
+        color_picker.set_color((0.5, 0.6, 0.7, 0.8))
+        self.assertEqual(color_picker.color, [0.5, 0.6, 0.7, 0.8])

kivy/uix/colorpicker.py

@@ -35,18 +35,19 @@ def on_color(instance, value):
 
 __all__ = ('ColorPicker', 'ColorWheel')
 
-from kivy.uix.relativelayout import RelativeLayout
-from kivy.uix.widget import Widget
+from math import cos, sin, pi, sqrt, atan
+from colorsys import rgb_to_hsv, hsv_to_rgb
+
+from kivy.clock import Clock
+from kivy.graphics import Mesh, InstructionGroup, Color
+from kivy.logger import Logger
 from kivy.properties import (NumericProperty, BoundedNumericProperty,
                              ListProperty, ObjectProperty,
                              ReferenceListProperty, StringProperty,
                              AliasProperty)
-from kivy.clock import Clock
-from kivy.graphics import Mesh, InstructionGroup, Color
+from kivy.uix.relativelayout import RelativeLayout
+from kivy.uix.widget import Widget
 from kivy.utils import get_color_from_hex, get_hex_from_color
-from kivy.logger import Logger
-from math import cos, sin, pi, sqrt, atan
-from colorsys import rgb_to_hsv, hsv_to_rgb
 
 
 def distance(pt1, pt2):
@@ -60,19 +61,19 @@ def polar_to_rect(origin, r, theta):
 def rect_to_polar(origin, x, y):
     if x == origin[0]:
         if y == origin[1]:
-            return (0, 0)
+            return 0, 0
         elif y > origin[1]:
-            return (y - origin[1], pi / 2.)
+            return y - origin[1], pi / 2
         else:
-            return (origin[1] - y, 3 * pi / 2.)
+            return origin[1] - y, 3 * pi / 2
     t = atan(float((y - origin[1])) / (x - origin[0]))
     if x - origin[0] < 0:
         t += pi
 
     if t < 0:
         t += 2 * pi
 
-    return (distance((x, y), origin), t)
+    return distance((x, y), origin), t
 
 
 class ColorWheel(Widget):
@@ -96,28 +97,28 @@ class ColorWheel(Widget):
     '''The Green value of the color currently selected.
 
     :attr:`g` is a :class:`~kivy.properties.BoundedNumericProperty`
-    and can be a value from 0 to 1.
+    and can be a value from 0 to 1. It defaults to 0.
     '''
 
     b = BoundedNumericProperty(0, min=0, max=1)
     '''The Blue value of the color currently selected.
 
     :attr:`b` is a :class:`~kivy.properties.BoundedNumericProperty` and
-    can be a value from 0 to 1.
+    can be a value from 0 to 1. It defaults to 0.
     '''
 
     a = BoundedNumericProperty(0, min=0, max=1)
     '''The Alpha value of the color currently selected.
 
     :attr:`a` is a :class:`~kivy.properties.BoundedNumericProperty` and
-    can be a value from 0 to 1.
+    can be a value from 0 to 1. It defaults to 0.
     '''
 
     color = ReferenceListProperty(r, g, b, a)
     '''The holds the color currently selected.
 
     :attr:`color` is a :class:`~kivy.properties.ReferenceListProperty` and
-    contains a list of `r`, `g`, `b`, `a` values.
+    contains a list of `r`, `g`, `b`, `a` values. It defaults to `[0, 0, 0, 0]`.
     '''
 
     _origin = ListProperty((100, 100))
@@ -132,22 +133,23 @@ class ColorWheel(Widget):
     _num_touches = 0
     _pinch_flag = False
 
-    _hsv = ListProperty([1, 1, 1, 0])
-
     def __init__(self, **kwargs):
-        super(ColorWheel, self).__init__(**kwargs)
+        self.arcs = []
+        self.sv_idx = 0
 
         pdv = self._piece_divisions
         self.sv_s = [(float(x) / pdv, 1) for x in range(pdv)] + [
             (1, float(y) / pdv) for y in reversed(range(pdv))]
 
-    def on__origin(self, instance, value):
-        self.init_wheel(None)
+        super(ColorWheel, self).__init__(**kwargs)
+
+    def on__origin(self, _instance, _value):
+        self._reset_canvas()
 
-    def on__radius(self, instance, value):
-        self.init_wheel(None)
+    def on__radius(self, _instance, _value):
+        self._reset_canvas()
 
-    def init_wheel(self, dt):
+    def _reset_canvas(self):
         # initialize list to hold all meshes
         self.canvas.clear()
         self.arcs = []
@@ -233,9 +235,8 @@ def on_touch_move(self, touch):
                             (float(self._radius) / self._piece_divisions)))
 
         if (
-            goal_sv_idx != self.sv_idx and
-            goal_sv_idx >= 0 and
-            goal_sv_idx <= len(self.sv_s) - self._piece_divisions
+                goal_sv_idx != self.sv_idx and
+                0 <= goal_sv_idx <= len(self.sv_s) - self._piece_divisions
         ):
             # this is a pinch to zoom
             self._pinch_flag = True
@@ -430,12 +431,12 @@ def _update_clr(self, dt):
         # to prevent interaction between hsv/rgba, we work internally using rgba
         mode, clr_idx, text = self._upd_clr_list
         try:
-            text = min(255, max(0, float(text)))
+            text = min(255.0, max(0.0, float(text)))
             if mode == 'rgb':
-                self.color[clr_idx] = float(text) / 255.
+                self.color[clr_idx] = text / 255
             else:
                 hsv = list(self.hsv[:])
-                hsv[clr_idx] = float(text) / 255.
+                hsv[clr_idx] = text / 255
                 self.color[:3] = hsv_to_rgb(*hsv)
         except ValueError:
             Logger.warning('ColorPicker: invalid value : {}'.format(text))