Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type hints? #74

Open
kylebarron opened this issue Apr 10, 2022 · 2 comments
Open

Type hints? #74

kylebarron opened this issue Apr 10, 2022 · 2 comments

Comments

@kylebarron
Copy link

👋

I saw mapbox/mercantile#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.

@sgillies
Copy link
Member

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.

@kylebarron
Copy link
Author

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":

@rasterio rasterio deleted a comment from kylebarron Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants