affine icon indicating copy to clipboard operation
affine copied to clipboard

Type hints?

Open kylebarron opened this issue 2 years ago • 2 comments

👋

I saw https://github.com/mapbox/mercantile/issues/140 and since @sgillies was receptive to the idea of adding type hints, I thought I'd also start a discussion here about adding type hints 🙂 .

Affine is a relatively small and self-contained library, so it could be a good library in the ecosystem to start with. I'd be happy to put up a PR if there's interest.

One possible hiccup that I found in the creation of external type stubs for affine is that mypy complained about violating the Liskov substitution principle. Specifically I wasn't able to type __mul__ to take an Affine object as input and output, because that's an incompatible override of the NamedTuple's __mul__ typing.

kylebarron avatar Apr 10 '22 19:04 kylebarron

Can the typing complaint not be ignored? I love how stable Affine is at the moment and would love to focus on other things and not make any new releases of it.

sgillies avatar Apr 10 '22 22:04 sgillies

A couple notes:

  • The first thing to check is if you have any interest in adding types to this repo. If not, that's totally fine.
  • Second, I agree that affine is wonderfully stable and I'm definitely not advocating for any code changes.
  • About the typing complaint, there are a couple ways to solve it:
    • We could use # type: ignore[override] to override the type error

    • Alternatively, to satisfy the condition, we just can't have the narrowest typing. So instead of

      def __mul__(self, other: "Affine") -> "Affine":
      

      instead we'd have

      def __mul__(self, other: namedtuple) -> "Affine":
      

kylebarron avatar Apr 14 '22 23:04 kylebarron