fix type errors (#3928)

* fix type checking error in overriding path propery

* fix type hints in sew stack

* enable type checking for tartan

* ignore type warnings for dynamic wx module attributes

* fix tartan type errors

* fix circular import

* add type-check and test targets

* use Optional instead

8 files changed, 92 insertions, 41 deletions

diff --git a/Makefile b/Makefile
index 4cf939c8..edb097b4 100644
--- a/Makefile
+++ b/Makefile
@@ -51,3 +51,11 @@ version:
 .PHONY: style
 style:
 	bash -x bin/style-check
+
+.PHONY: type-check mypy
+type-check mypy:
+	python -m mypy
+
+.PHONY: test
+test:
+	pytest
diff --git a/lib/sew_stack/stitch_layers/mixins/path.py b/lib/sew_stack/stitch_layers/mixins/path.py
index 88e5419d..9739f381 100644
--- a/lib/sew_stack/stitch_layers/mixins/path.py
+++ b/lib/sew_stack/stitch_layers/mixins/path.py
@@ -1,6 +1,6 @@
+from .protocol import LayerProtocol
 from ..stitch_layer_editor import Category, Property
 from ....i18n import _
-from ....utils import DotDict, Point
 
 
 class PathPropertiesMixin:
@@ -13,10 +13,7 @@ class PathPropertiesMixin:
 
 
 class PathMixin:
-    config: DotDict
-    paths: 'list[list[Point]]'
-
-    def get_paths(self):
+    def get_paths(self: LayerProtocol):
         paths = self.paths
 
         if self.config.reverse_path:
diff --git a/lib/sew_stack/stitch_layers/mixins/protocol.py b/lib/sew_stack/stitch_layers/mixins/protocol.py
new file mode 100644
index 00000000..1057bd7b
--- /dev/null
+++ b/lib/sew_stack/stitch_layers/mixins/protocol.py
@@ -0,0 +1,50 @@
+from typing import Protocol, TYPE_CHECKING
+
+from lib.utils import DotDict, Point
+
+
+if TYPE_CHECKING:
+    from lib.sew_stack import SewStack
+
+
+class LayerProtocol(Protocol):
+    paths: 'list[list[Point]]'
+    config: DotDict
+    element: 'SewStack'
+
+
+def with_protocol(protocol):
+    """Include a protocol in a Mixin only for type hinting.
+
+    Normally we'd use a protocol in a mixin to indicate that we
+    expect certain attributes to be available on self, as
+    described here:
+
+    https://mypy.readthedocs.io/en/stable/more_types.html#mixin-classes
+
+    However, for some reason type-checkers then _only_ allow use of
+    properties on self that are defined in the protocol.  For example,
+    this doesn't work:
+
+    class MyMixin:
+        def foo(self):
+            return "hi"
+
+        def bar(self: LayerProtocol):
+            thing = self.foo()
+            if self.config.baz == thing:
+                ...
+
+    This fails because mypy says that "self.foo()" references unknown
+    attribute "foo"... even though it's defined right there in MyMixin.
+    This feels a little weird, but that's how it works.
+
+    Instead, we do it like this:
+
+    class MyMixin(with_protocol(LayerProtocol)):
+        ...
+    """
+    if TYPE_CHECKING:
+        return protocol
+    else:
+        return object
diff --git a/lib/sew_stack/stitch_layers/mixins/randomization.py b/lib/sew_stack/stitch_layers/mixins/randomization.py
index 5414731c..009b5042 100644
--- a/lib/sew_stack/stitch_layers/mixins/randomization.py
+++ b/lib/sew_stack/stitch_layers/mixins/randomization.py
@@ -1,17 +1,13 @@
 import os
 from secrets import randbelow
-from typing import TYPE_CHECKING
 
 import wx.propgrid
 
+from .protocol import LayerProtocol, with_protocol
 from ..stitch_layer_editor import Category, Property
 from ....i18n import _
 from ....svg import PIXELS_PER_MM
-from ....utils import DotDict, get_resource_dir, prng
-
-if TYPE_CHECKING:
-    from ... import SewStack
-
+from ....utils import get_resource_dir, prng
 
 editor_instance = None
 
@@ -70,10 +66,7 @@ class RandomizationPropertiesMixin:
         )
 
 
-class RandomizationMixin:
-    config: DotDict
-    element: "SewStack"
-
+class RandomizationMixin(with_protocol(LayerProtocol)):
     @classmethod
     def randomization_defaults(cls):
         return dict(
diff --git a/lib/tartan/palette.py b/lib/tartan/palette.py
index b8c36331..09dfb373 100644
--- a/lib/tartan/palette.py
+++ b/lib/tartan/palette.py
@@ -64,7 +64,7 @@ class Palette:
                 stripe = {'render': 1, 'color': '#000000', 'width': '5'}
                 stripe_panel = cast('StripePanel', stripe_sizer.GetWindow())
                 stripe['render'] = stripe_panel.visibility.Get3StateValue()
-                stripe['color'] = stripe_panel.colorpicker.GetColour().GetAsString(wx.C2S_HTML_SYNTAX)
+                stripe['color'] = stripe_panel.colorpicker.GetColour().GetAsString(wx.C2S_HTML_SYNTAX)  # type: ignore[attr-defined]
                 stripe['width'] = stripe_panel.stripe_width.GetValue()
                 stripes.append(stripe)
             self.palette_stripes[i] = stripes
@@ -173,8 +173,8 @@ class Palette:
                 try:
                     # on macOS we need to run wxpython color method inside the app otherwise
                     # the color picker has issues in some cases to accept our input
-                    color = wx.Colour(color).GetAsString(wx.C2S_HTML_SYNTAX)
-                except wx.PyNoAppError:
+                    color = wx.Colour(color).GetAsString(wx.C2S_HTML_SYNTAX)  # type: ignore[attr-defined]
+                except wx.PyNoAppError:  # type: ignore[attr-defined]
                     # however when we render an embroidery element we do not want to open wx.App
                     try:
                         color = str(Color(color).to_named())
diff --git a/lib/tartan/svg.py b/lib/tartan/svg.py
index 1fd4b3ad..7841247e 100644
--- a/lib/tartan/svg.py
+++ b/lib/tartan/svg.py
@@ -66,6 +66,10 @@ class TartanSvgGroup:
         :param outline: the outline to be filled with the tartan pattern
         """
         parent_group = outline.getparent()
+
+        if parent_group is None:
+            raise ValueError("outline must have a parent")
+
         if parent_group is not None and parent_group.get_id().startswith('inkstitch-tartan'):
             # remove everything but the tartan outline
             for child in parent_group.iterchildren():
@@ -202,10 +206,10 @@ class TartanSvgGroup:
         :param fill_stitch_graph: the stitch graph
         :param polygons: the polygon shapes (if not LineStrings)
         :param geometry_type: wether to render 'polygon' or 'linestring' segments
-        :param outline_shape: the shape to be filkled with the tartan pattern
+        :param outline_shape: the shape to be filled with the tartan pattern
         :returns: a list of routed shape elements
         """
-        outline = MultiLineString()
+        outline = LineString()
         travel_linestring = LineString()
         routed_shapes = []
         start_distance = 0
@@ -232,7 +236,11 @@ class TartanSvgGroup:
                     travel_linestring = self._get_travel(start, end, outline)
                 else:
                     end_distance = outline.project(Point(end))
-                    travel_linestring = substring(outline, start_distance, end_distance)
+                    result = substring(outline, start_distance, end_distance)
+                    if isinstance(result, Point):
+                        travel_linestring = LineString()
+                    else:
+                        travel_linestring = result
         return routed_shapes
 
     def _edge_segment_to_element(
@@ -281,16 +289,16 @@ class TartanSvgGroup:
         """
         if outline.length / 2 < travel_linestring.length:
             short_travel = outline.difference(travel_linestring)
-            if short_travel.geom_type == "MultiLineString":
+            if isinstance(short_travel, MultiLineString):
                 short_travel = linemerge(short_travel)
-            if short_travel.geom_type == "LineString":
+            if isinstance(short_travel, LineString):
                 if Point(short_travel.coords[-1]).distance(Point(start)) > Point(short_travel.coords[0]).distance(Point(start)):
                     short_travel = reverse(short_travel)
                 return short_travel
         return travel_linestring
 
     @staticmethod
-    def _find_polygon(polygons: List[Polygon], point: Tuple[float, float]) -> Optional[Polygon]:
+    def _find_polygon(polygons: List[Polygon], point: Point) -> Optional[Polygon]:
         """
         Find the polygon for a given point
 
@@ -449,7 +457,12 @@ class TartanSvgGroup:
         """
         start_distance = outline.project(Point(start))
         end_distance = outline.project(Point(end))
-        return substring(outline, start_distance, end_distance)
+
+        result = substring(outline, start_distance, end_distance)
+        if isinstance(result, Point):
+            return LineString((result, result))
+        else:
+            return result
 
     def _get_dimensions(self, outline: MultiPolygon) -> Tuple[Tuple[float, float, float, float], Point]:
         """
@@ -485,7 +498,7 @@ class TartanSvgGroup:
         transform: str,
         start: Optional[Tuple[float, float]] = None,
         end: Optional[Tuple[float, float]] = None
-    ) -> Optional[PathElement]:
+    ) -> PathElement:
         """
         Convert a polygon to an svg path element
 
@@ -499,8 +512,6 @@ class TartanSvgGroup:
         """
         path = Path(list(polygon.exterior.coords))
         path.close()
-        if path is None:
-            return None
 
         for interior in polygon.interiors:
             interior_path = Path(list(interior.coords))
diff --git a/lib/tartan/utils.py b/lib/tartan/utils.py
index 7949505d..bf31a636 100644
--- a/lib/tartan/utils.py
+++ b/lib/tartan/utils.py
@@ -6,7 +6,7 @@
 import json
 from collections import defaultdict
 from copy import copy
-from typing import List, Tuple, Union
+from typing import List, Optional, Tuple, Union
 
 from inkex import BaseElement
 from shapely import LineString, MultiPolygon, Point, Polygon, unary_union
@@ -81,7 +81,6 @@ def stripes_to_shapes(
                 shapes[stripe['color']].append(polygon)
                 left = right
                 top = bottom
-    return shapes
 
 
 def _stripes_to_sett(
@@ -100,7 +99,7 @@ def _stripes_to_sett(
     :returns: a list of dictionaries with stripe information (color, width, is_stroke, render)
     """
 
-    last_fill_color = _get_last_fill_color(stripes, scale, min_stripe_width, symmetry)
+    last_fill_color: Optional[str] = _get_last_fill_color(stripes, scale, min_stripe_width, symmetry)
     first_was_stroke = False
     last_was_stroke = False
     add_width = 0
@@ -152,7 +151,7 @@ def _stripes_to_sett(
     return sett
 
 
-def _get_last_fill_color(stripes: List[dict], scale: int, min_stripe_width: float, symmetry: bool,) -> List[dict]:
+def _get_last_fill_color(stripes: List[dict], scale: int, min_stripe_width: float, symmetry: bool,) -> Optional[str]:
     '''
     Returns the first fill color of a pattern to substitute spaces if the pattern starts with strokes or
     stripes with render mode 2
@@ -163,7 +162,7 @@ def _get_last_fill_color(stripes: List[dict], scale: int, min_stripe_width: floa
     :param symmetry: reflective sett (True) / repeating sett (False)
     :returns: a list with fill colors or a list with one None item if there are no fills
     '''
-    fill_colors = []
+    fill_colors: list[Optional[str]] = []
     for stripe in stripes:
         if stripe['render'] == 0:
             fill_colors.append(None)
@@ -238,7 +237,7 @@ def _get_linestrings(
     dimensions: List[float],
     rotation: float,
     rotation_center: Point, weft: bool
-) -> list:
+) -> List[LineString]:
     """
     Generates a rotated linestrings with the given dimension (outline intersection)
 
diff --git a/mypy.ini b/mypy.ini
index 3d394885..069ca8ff 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -21,13 +21,6 @@ check_untyped_defs = True
 disallow_incomplete_defs = True
 disallow_untyped_defs = True
 
-# This part of the code will need some work before it'll start passing.
-[mypy-lib.tartan.*]
-ignore_errors = True
-
-[mypy-lib.sew_stack.*]
-ignore_errors = True
-
 [mypy-tests.*]
 # The tests should be typechecked because they're all new code, and because they're tests we don't really care if they have perfect annotations.
 check_untyped_defs = True