Add type annotations to fontshell. (#826)

* Annotate `fontshell`` modules.

• Fix related inconsistencies in base classes.

* Annotate more modules and correct base classes.

* Annotate more modules.

* Replace `getNaked` function with `type: ignore` in
`naked`.

- Various other fixes.

* Correct `_get_color` return and mypy errors

* Correct mypy errors

* Change to `assertCountEqual` for keys in `test_copy`

* Correct more mypy errors.

* Update error message for None component check

---------

Co-authored-by: knutnergaard <15194233+knutnergaard@users.noreply.github.com>
Co-authored-by: Ben Kiel <ben@benkiel.com>

Author: knutnergaard

Lib/fontParts/base/base.py

@@ -25,7 +25,6 @@
 from fontParts.base import normalizers
 from fontParts.base.annotations import (
     PairType,
-    CollectionType,
     PairCollectionType,
     SextupleCollectionType,
     IntFloatType,

Lib/fontParts/base/font.py

@@ -131,8 +131,8 @@ def copyData(self, source: BaseFont) -> None:
             else:
                 layer = self.newLayer(layerName)
             layer.copyData(source.getLayer(layerName))
-        for guideline in self.guidelines:
-            self.appendGuideline(guideline)
+        for guideline in source.guidelines:
+            self.appendGuideline(guideline=guideline)
         super(BaseFont, self).copyData(source)
 
     # ---------------
@@ -293,7 +293,7 @@ def _save(
         self,
         path: Optional[str],
         showProgress: bool,
-        formatVersion: Optional[float],
+        formatVersion: Optional[int],
         fileStructure: Optional[str],
         **kwargs: Any,
     ) -> None:
@@ -1828,6 +1828,8 @@ def appendGuideline(
                     identifier = normalizedGuideline.identifier
         if position is not None:
             position = normalizers.normalizeCoordinateTuple(position)
+        else:
+            raise ValueError("Position cannot be None.")
         if angle is not None:
             angle = normalizers.normalizeRotationAngle(angle)
         if name is not None:
@@ -1843,11 +1845,11 @@ def appendGuideline(
 
     def _appendGuideline(  # type: ignore[return]
         self,
-        position: Optional[PairCollectionType[IntFloatType]],
+        position: PairCollectionType[IntFloatType],
         angle: Optional[float],
         name: Optional[str],
         color: Optional[QuadrupleCollectionType[IntFloatType]],
-        **kwargs,
+        **kwargs: Any,
     ) -> BaseGuideline:
         r"""Append a new guideline to the native font.
 

Lib/fontParts/base/glyph.py

@@ -308,7 +308,7 @@ def _set_base_unicodes(self, value: CollectionType[int]) -> None:
         value = normalizers.normalizeGlyphUnicodes(value)
         self._set_unicodes(value)
 
-    def _get_unicodes(self) -> Tuple[int, ...]:  # type: ignore[return]
+    def _get_unicodes(self) -> CollectionType[int]:  # type: ignore[return]
         """Get the Unicode values assigned to the glyph.
 
         This is the environment implementation of
@@ -3226,16 +3226,18 @@ def _removeLayer(self, name: str, **kwargs: Any) -> None:
         """,
     )
 
-    def _get_base_image(self) -> BaseImage:
+    def _get_base_image(self) -> Optional[BaseImage]:
         image = self._get_image()
-        if image.glyph is None:
-            image.glyph = self
+        if image is not None:
+            if image.glyph is None:
+                image.glyph = self
         return image
 
-    def _get_image(self) -> BaseImage:  # type: ignore[return]
+    def _get_image(self) -> Optional[BaseImage]:  # type: ignore[return]
         """Get the image for the native glyph.
 
-        :return: The :class:`BaseImage` subclass instance belonging to the glyph.
+        :return: The :class:`BaseImage` subclass instance belonging to the glyph,
+            or :obj:`None` if the glyph has no image.
         :raises NotImplementedError: If the method has not been overridden by a
             subclass.
 

Lib/fontParts/base/groups.py

@@ -9,9 +9,9 @@
 
 if TYPE_CHECKING:
     from fontParts.base.font import BaseFont
-    from fontparts.base import BaseItems
-    from fontparts.base import BaseKeys
-    from fontparts.base import BaseValues
+    from fontParts.base.base import BaseKeys
+    from fontParts.base.base import BaseItems
+    from fontParts.base.base import BaseValues
 
 ValueType = Tuple[str, ...]
 GroupsDict = Dict[str, ValueType]
@@ -410,7 +410,7 @@ def get(
         """
         return super(BaseGroups, self).get(groupName, default)
 
-    def items(self) -> BaseItems[Tuple[str, ValueType]]:
+    def items(self) -> BaseItems[str, ValueType]:
         """Return the items in the current groups instance.
 
         Each item is represented as a :class:`tuple` of key-value pairs, where:

Lib/fontParts/base/guideline.py

@@ -535,7 +535,7 @@ def _set_name(self, value: Optional[str]) -> None:
         """,
     )
 
-    def _get_base_color(self) -> Optional[Color]:
+    def _get_base_color(self) -> Optional[QuadrupleType[float]]:
         value = self._get_color()
         if value is not None:
             value = Color(value)
@@ -548,7 +548,7 @@ def _set_base_color(
             value = normalizers.normalizeColor(value)
         self._set_color(value)
 
-    def _get_color(self) -> Optional[QuadrupleCollectionType[IntFloatType]]:
+    def _get_color(self) -> Optional[QuadrupleType[float]]:
         """Get the native guideline's color.
 
         This is the environment implementation of the :attr:`BaseGuideline.color`

Lib/fontParts/base/image.py

@@ -420,24 +420,26 @@ def _set_color(self, value: Optional[QuadrupleType[float]]) -> None:
         The possible formats are defined by each environment.
         The value must be a :class:`bytes` object.
 
-        :return: A :class:`bytes` object representing the raw byte data of the image.
+        :return: A :class:`bytes` object representing the raw byte data of the image 
+            or :obj:`None`.
 
         """,
     )
 
-    def _get_base_data(self) -> bytes:
+    def _get_base_data(self) -> Optional[bytes]:
         return self._get_data()
 
     def _set_base_data(self, value: bytes) -> None:
         self._set_data(value)
 
-    def _get_data(self) -> bytes:
+    def _get_data(self) -> Optional[bytes]:
         """Get the native image's raw byte data.
 
         This is the environment implementation of the :attr:`BaseImage.data`
         property getter.
 
-        :return: A :class:`bytes` object representing the data of the image.
+        :return: A :class:`bytes` object representing the raw byte data of the image
+            or :obj:`None`.
         :raises NotImplementedError: If the method has not been overridden by a
             subclass.
 

Lib/fontParts/base/layer.py

@@ -804,18 +804,20 @@ def _set_name(self, value: str, **kwargs: Any) -> None:
         """,
     )
 
-    def _get_base_color(self) -> QuadrupleCollectionType[IntFloatType]:
+    def _get_base_color(self) -> Optional[QuadrupleCollectionType[IntFloatType]]:
         value = self._get_color()
         if value is not None:
             value = Color(value)
         return value
 
-    def _set_base_color(self, value: QuadrupleCollectionType[IntFloatType]) -> None:
+    def _set_base_color(
+        self, value: Optional[QuadrupleCollectionType[IntFloatType]]
+    ) -> None:
         if value is not None:
             value = normalizers.normalizeColor(value)
         self._set_color(value)
 
-    def _get_color(self) -> QuadrupleCollectionType[IntFloatType]:  # type: ignore[return]
+    def _get_color(self) -> Optional[QuadrupleCollectionType[IntFloatType]]:  # type: ignore[return]
         """Get the color of the layer.
 
         This is the environment implementation of
@@ -835,7 +837,7 @@ def _get_color(self) -> QuadrupleCollectionType[IntFloatType]:  # type: ignore[r
         self.raiseNotImplementedError()
 
     def _set_color(
-        self, value: QuadrupleCollectionType[IntFloatType], **kwargs: Any
+        self, value: Optional[QuadrupleCollectionType[IntFloatType]], **kwargs: Any
     ) -> None:
         r"""Get or set the color of the layer.
 

Lib/fontParts/base/lib.py

@@ -10,10 +10,10 @@
 if TYPE_CHECKING:
     from fontParts.base.glyph import BaseGlyph
     from fontParts.base.font import BaseFont
-    from fontparts.base.layer import BaseLayer
-    from fontparts.base import BaseItems
-    from fontparts.base import BaseKeys
-    from fontparts.base import BaseValues
+    from fontParts.base.layer import BaseLayer
+    from fontParts.base.base import BaseItems
+    from fontParts.base.base import BaseValues
+    from fontParts.base.base import BaseKeys
 
 
 class BaseLib(BaseDict, DeprecatedLib, RemovedLib):

Lib/fontParts/fontshell/anchor.py

@@ -1,36 +1,45 @@
+from __future__ import annotations
+from typing import Optional
+
 import defcon
 from fontParts.base import BaseAnchor
+from fontParts.base.annotations import (
+    QuadrupleType,
+    QuadrupleCollectionType,
+    IntFloatType,
+)
 from fontParts.fontshell.base import RBaseObject
 
 
 class RAnchor(RBaseObject, BaseAnchor):
     wrapClass = defcon.Anchor
 
-    def _init(self, wrap=None):
-        if wrap is None:
-            wrap = self.wrapClass()
-            wrap.x = 0
-            wrap.y = 0
-        super(RAnchor, self)._init(wrap=wrap)
+    def _init(self, pathOrObject: Optional[defcon.Anchor] = None) -> None:
+        if self.wrapClass is not None:
+            if pathOrObject is None:
+                pathOrObject = self.wrapClass()
+                pathOrObject.x = 0
+                pathOrObject.y = 0
+            super(RAnchor, self)._init(pathOrObject=pathOrObject)
 
     # --------
     # Position
     # --------
 
     # x
 
-    def _get_x(self):
+    def _get_x(self) -> float:
         return self.naked().x
 
-    def _set_x(self, value):
+    def _set_x(self, value: float) -> None:
         self.naked().x = value
 
     # y
 
-    def _get_y(self):
+    def _get_y(self) -> float:
         return self.naked().y
 
-    def _set_y(self, value):
+    def _set_y(self, value: float) -> None:
         self.naked().y = value
 
     # --------------
@@ -39,32 +48,32 @@ def _set_y(self, value):
 
     # identifier
 
-    def _get_identifier(self):
-        anchor = self.naked()
-        return anchor.identifier
+    def _get_identifier(self) -> Optional[str]:
+        return self.naked().identifier
 
-    def _getIdentifier(self):
-        anchor = self.naked()
-        return anchor.generateIdentifier()
+    def _getIdentifier(self) -> str:
+        return self.naked().generateIdentifier()
 
-    def _setIdentifier(self, value):
+    def _setIdentifier(self, value: str) -> None:
         self.naked().identifier = value
 
     # name
 
-    def _get_name(self):
+    def _get_name(self) -> Optional[str]:
         return self.naked().name
 
-    def _set_name(self, value):
+    def _set_name(self, value: Optional[str]) -> None:
         self.naked().name = value
 
     # color
 
-    def _get_color(self):
+    def _get_color(self) -> Optional[QuadrupleType[float]]:
         value = self.naked().color
         if value is not None:
             value = tuple(value)
         return value
 
-    def _set_color(self, value):
+    def _set_color(
+        self, value: Optional[QuadrupleCollectionType[IntFloatType]]
+    ) -> None:
         self.naked().color = value

Lib/fontParts/fontshell/base.py

@@ -1,16 +1,23 @@
-class RBaseObject(object):
-    wrapClass = None
+from __future__ import annotations
+from typing import Generic, Optional, Type, TypeVar
 
-    def _init(self, wrap=None):
-        if wrap is None and self.wrapClass is not None:
-            wrap = self.wrapClass()
-        if wrap is not None:
-            self._wrapped = wrap
+RBaseObjectType = TypeVar("RBaseObjectType", bound="RBaseObject")
 
-    def changed(self):
+
+class RBaseObject(Generic[RBaseObjectType]):
+    wrapClass: Optional[Type[RBaseObjectType]] = None
+    dirty: bool
+
+    def _init(self, pathOrObject: Optional[RBaseObjectType] = None) -> None:
+        if pathOrObject is None and self.wrapClass is not None:
+            pathOrObject = self.wrapClass()  # pylint: disable=E1102
+        if pathOrObject is not None:
+            self._wrapped = pathOrObject
+
+    def changed(self) -> None:
         self.naked().dirty = True
 
-    def naked(self):
+    def naked(self) -> RBaseObjectType:
         if hasattr(self, "_wrapped"):
             return self._wrapped
-        return None
+        return None  # type: ignore[return-value]