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

Complete type hints #121

Merged
merged 4 commits into from
Jan 26, 2025
Merged

Complete type hints #121

merged 4 commits into from
Jan 26, 2025

Conversation

sgillies
Copy link
Member

Resolves #74

@sgillies sgillies added this to the 3.0 milestone Jan 25, 2025
@sgillies sgillies self-assigned this Jan 25, 2025
`a`, `b`, and `c` are the elements of the first row of the
matrix. `d`, `e`, and `f` are the elements of the second row.
Coefficients of the 3 x 3 augmented affine transformation
matrix.
Copy link
Member Author

@sgillies sgillies Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to type hinting: I removed a duplicated diagram.

src/affine.py Outdated
def from_gdal(cls, c: float, a: float, b: float, f: float, d: float, e: float):
def from_gdal(
cls, c: float, a: float, b: float, f: float, d: float, e: float
) -> "Affine":
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy was fine with Affine, but ruff insists on quotes 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a typing pro, but after a 20 second read of PEP 649 it is potentially related. Add from __future__ import annotations to the top which should make ruff happy.

src/affine.py Outdated
@@ -281,7 +280,7 @@ def permutation(cls, *scaling):
"""
return cls(0.0, 1.0, 0.0, 1.0, 0.0, 0.0)

def __array__(self, dtype=None, copy=None):
def __array__(self, dtype=None, copy: Optional[bool] = None):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm skipping the type hint for dtype, to avoid a dependency on numpy.

)
assert not isinstance(other, Affine)
return self.__mul__(other)
return NotImplemented
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not related to type hinting: removal of the right multiplication implementation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note: I'm not providing type hints for dunder methods. These are specified by the Python language.

@sgillies
Copy link
Member Author

What do you think @mwtoews ?

@mwtoews
Copy link
Contributor

mwtoews commented Jan 25, 2025

You could add Typing :: Typed to pyproject.toml#L21

@mwtoews
Copy link
Contributor

mwtoews commented Jan 25, 2025

There are potentially a few more to define via:

ruff check --select ANN --ignore "ANN202,ANN204" src

But honestly, it looks good enough for me.

@sgillies sgillies merged commit 756471b into main Jan 26, 2025
7 checks passed
@sgillies sgillies deleted the issue74 branch January 26, 2025 00:59
@Rotzbua Rotzbua mentioned this pull request Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type hints?
2 participants