Revise type annotations slightly #1860

EliahKagan · 2024-03-05T01:42:00Z

This pull request is mostly a small revision to type hints to improve a couple things I had noticed when working on #1850 but omitted from there to avoid bloating its scope:

9f226fc: __slots__ is typically unannotated, but in once place it had an unnecessary annotation.
5d7e55b: Although mypy treats it the same when there are parameter annotations, __init__ is best annotated with an explicit return type of None, per PEP 484.

In addition, I noticed some slight under-indentation in a few docstrings I had worked on #1850, and fixed that here in e984bfe. (The type annotation tweaks are sufficiently minor that there is an argument to be made that this is really the most significant individual change in this pull request.)

There is some more information in the commit messages.

The correct type is inferred, and no other __slots__ variable in the codebase has an annotation.

A few docstrings had parts that were meant to be indented the usual four spaces beyond the surrounding indentation, but were indented only three spaces beyond it instead.

Although sometimes found unintuitive, the reutrn type of a class's __init__ method is best annotated as None, as in other functions that return implicitly or by operand-less return statements. Note that this is only a minor improvement, effectively just a style fix, because mypy treats __init__ specially and, *when at least one parameter is annotated*, its return type is implicitly None rather than implicitly Any like other functions. All the __init__ methods modified here did already have one or more annotated parameters. However, having __init__ methods without the return type makes it easier to introduce a bug where an __init__ method with no parameters besides self -- which itself should almost always be unannotated and is properly inferred -- is written and the return annotation needed to get mypy to regard it as statically typed, so it checks it at all, is omitted. (It is also inelegant when one considers the meaning of __init__ and the distinction between it and __new__.) This commit does not add any return type annotations to functions in the test suite, since the test suite doesn't currently use static typing. Further reading: - https://peps.python.org/pep-0484/#the-meaning-of-annotations - python/mypy#604 - gitpython-developers#1755 (comment)

Byron

Thanks a lot!

I really am impressed by the diligence needed to come up with all of these fixes (particularly in a codebase which might not be the most pleasant to read given its age and moved history).

EliahKagan added 3 commits March 4, 2024 20:10

Remove unneeded annotation on __slots__ variable

9f226fc

The correct type is inferred, and no other __slots__ variable in the codebase has an annotation.

Fix some underindented portions of docstrings

e984bfe

A few docstrings had parts that were meant to be indented the usual four spaces beyond the surrounding indentation, but were indented only three spaces beyond it instead.

EliahKagan marked this pull request as ready for review March 5, 2024 01:43

Byron approved these changes Mar 5, 2024

View reviewed changes

Byron merged commit 12c139c into gitpython-developers:main Mar 5, 2024
26 checks passed

EliahKagan deleted the typing-tweaks branch March 5, 2024 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise type annotations slightly #1860

Revise type annotations slightly #1860

EliahKagan commented Mar 5, 2024

Byron left a comment

Revise type annotations slightly #1860

Revise type annotations slightly #1860

Conversation

EliahKagan commented Mar 5, 2024

Byron left a comment

Choose a reason for hiding this comment