GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Git.{AutoInterrupt,CatFileContentStream} refactor broke static typing · Issue #2038 · gitpython-developers/GitPython

Git.{AutoInterrupt,CatFileContentStream} refactor broke static typing #2038

New issue

Copy link

New issue

Copy link

Closed

#2039

Closed

Git.{AutoInterrupt,CatFileContentStream} refactor broke static typing#2038

#2039

Copy link

Description

EliahKagan

opened

on Jun 7, 2025

Issue body actions

#2037 accidentally introduced 30 new mypy errors, more than doubling the total number that occur on most recent versions of Python from 27 to 57 (or from 28 to 58 in Python 3.13):

diff --git a/a b/b
index 128f3547..59913c9c 100644
--- a/a
+++ b/b
@@ -1,33 +1,82 @@
 git\types.py:133: error: Type[...] can't contain "Union[Literal[...], Literal[...]]"  [valid-type]
+git\util.py:508: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\util.py:508: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\util.py:512: error: Item Git.AutoInterrupt? of "Union[Popen[Any], Git.AutoInterrupt?]" has no attribute "wait"  [union-attr]
 git\util.py:1146: error: Variance of TypeVar "T_IterableObj" incompatible with variance in parent type  [type-var]
 git\util.py:1217: error: Argument 2 to "getattr" has incompatible type "Union[SupportsIndex, str]"; expected "str"  [arg-type]
 git\util.py:1219: error: Unsupported operand types for + ("str" and "SupportsIndex")  [operator]
 git\util.py:1219: note: Right operand is of type "Union[SupportsIndex, str]"
 git\util.py:1226: error: Unsupported operand types for + ("str" and "SupportsIndex")  [operator]
 git\util.py:1226: error: Unsupported operand types for + ("str" and "slice[Any, Any, Any]")  [operator]
 git\util.py:1226: note: Right operand is of type "Union[SupportsIndex, slice[Any, Any, Any], str]"
 git\config.py:572: error: If x = b'abc' then f"{x}" or "{}".format(x) produces "b'abc'", not "abc". If this is desired behavior, use f"{x!r}" or "{!r}".format(x). Otherwise, decode the bytes  [str-bytes-safe]
 git\config.py:628: error: Redundant cast to "Union[str, PathLike[str]]"  [redundant-cast]
+git\cmd.py:98: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:98: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:106: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:106: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:170: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:170: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:218: error: Unused "type: ignore" comment  [unused-ignore]
+git\cmd.py:510: error: Variable "git.cmd.Git.CatFileContentStream" is not valid as a type  [valid-type]
+git\cmd.py:510: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1051: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1051: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1108: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1108: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1528: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1528: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1533: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1533: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1537: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1537: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1658: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1658: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1668: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1668: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1671: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\cmd.py:1671: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1672: error: Git.AutoInterrupt? has no attribute "stdin"  [attr-defined]
+git\cmd.py:1672: error: Git.AutoInterrupt? has no attribute "stdout"  [attr-defined]
+git\cmd.py:1673: error: Git.AutoInterrupt? has no attribute "stdin"  [attr-defined]
+git\cmd.py:1674: error: Git.AutoInterrupt? has no attribute "stdin"  [attr-defined]
+git\cmd.py:1675: error: Git.AutoInterrupt? has no attribute "stdout"  [attr-defined]
+git\cmd.py:1703: error: Git.CatFileContentStream? has no attribute "read"  [attr-defined]
+git\cmd.py:1707: error: Variable "git.cmd.Git.CatFileContentStream" is not valid as a type  [valid-type]
+git\cmd.py:1707: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\cmd.py:1719: error: Git.AutoInterrupt? has no attribute "stdout"  [attr-defined]
 git\refs\log.py:148: error: Argument 1 to "RefLogEntry" has incompatible type "Tuple[str, str, Actor, Tuple[int, int], Optional[str]]"; expected "Iterable[Union[str, Actor, Tuple[int, int]]]"  [arg-type]
+git\diff.py:584: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\diff.py:584: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
 git\objects\submodule\base.py:834: error: "SymbolicReference" has no attribute "set_tracking_branch"  [attr-defined]
 git\objects\submodule\base.py:862: error: "SymbolicReference" has no attribute "tracking_branch"  [attr-defined]
+git\objects\commit.py:454: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\objects\commit.py:454: note: Error code "valid-type" not covered by "type: ignore" comment
+git\objects\commit.py:454: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\objects\commit.py:459: error: Git.AutoInterrupt? has no attribute "communicate"  [attr-defined]
 git\objects\commit.py:711: error: Argument 2 to "create" of "SymbolicReference" has incompatible type "Callable[[], SymbolicReference]"; expected "Union[str, PathLike[str]]"  [arg-type]
 git\objects\commit.py:903: error: Argument 2 to "findall" has incompatible type "Union[str, bytes]"; expected "str"  [arg-type]
 git\refs\symbolic.py:919: error: Incompatible types in assignment (expression has type "SymbolicReference", variable has type "T_References")  [assignment]
 git\refs\tag.py:48: error: Read-only property cannot override read-write property  [misc]
 git\refs\tag.py:49: error: Unused "type: ignore" comment  [unused-ignore]
 git\refs\tag.py:83: error: Read-only property cannot override read-write property  [misc]
 git\refs\tag.py:84: error: Unused "type: ignore" comment  [unused-ignore]
 git\refs\head.py:48: error: Incompatible override of a setter type  [override]
 git\refs\head.py:48: note:  (base class "SymbolicReference" defined the type as "Union[Commit, SymbolicReference, str]",
 git\refs\head.py:48: note:  override has type "Commit")
 git\refs\head.py:48: note:  Setter types should behave contravariantly
+git\remote.py:874: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\remote.py:874: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\remote.py:902: error: Git.AutoInterrupt? has no attribute "wait"  [attr-defined]
+git\remote.py:945: error: Variable "git.cmd.Git.AutoInterrupt" is not valid as a type  [valid-type]
+git\remote.py:945: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
+git\remote.py:975: error: Git.AutoInterrupt? has no attribute "wait"  [attr-defined]
 git\repo\fun.py:289: error: "Callable[[], SymbolicReference]" has no attribute "commit"  [attr-defined]
 git\repo\fun.py:339: error: "Callable[[], SymbolicReference]" has no attribute "log_entry"  [attr-defined]
 git\repo\base.py:688: error: Redundant cast to "Literal['system', 'global', 'user', 'repository']"  [redundant-cast]
 git\repo\base.py:690: error: Redundant cast to "Literal['system', 'global', 'user', 'repository']"  [redundant-cast]
 git\repo\base.py:1054: error: Incompatible return value type (got "SymbolicReference", expected "Head")  [return-value]
 git\repo\base.py:1487: error: Argument 5 to "_clone" of "Repo" has incompatible type "Optional[Optional[Callable[[int, Union[str, float], Union[str, float, None], str], None]]]"; expected "Union[RemoteProgress, UpdateProgress, Callable[..., RemoteProgress], None]"  [arg-type]
 git\repo\base.py:1548: error: Argument 5 to "_clone" of "Repo" has incompatible type "Optional[Callable[[int, Union[str, float], Union[str, float, None], str], None]]"; expected "Union[RemoteProgress, UpdateProgress, Callable[..., RemoteProgress], None]"  [arg-type]
 test\deprecation\test_basic.py:137: error: Argument "change_type" to "iter_change_type" of "DiffIndex" has incompatible type "str"; expected "Literal['A', 'D', 'C', 'M', 'R', 'T', 'U']"  [arg-type]
-Found 27 errors in 12 files (checked 44 source files)
+Found 57 errors in 15 files (checked 44 source files)

The problem is that, as described in Variables vs type aliases, mypy only recognizes assignments to create type aliases when they are a module level, when TypeAlias is used, or when the Python 3.12+ type syntax is used. This is at class level, TypeAlias is not used, and type is not used (and can't be used, because we support versions of Python below 3.12).

So even though Sphinx would readily recognize these as type aliases (hence the need to change __name__ and __qualname__) when we didn't want it to do so, mypy would not recognize them as type aliases even though we do want to either to do so (or to recognize them as nested classes, but I don't think that's feasible to have a static type-checker do, and it may not be desirable).

This will actually affect users unless it is fixed before the next release is made, because Git.AutoInterrupt and Git.CatFileContentStream are public types that users are supposed to be able to access directly.

I'm aware of several possible solutions, some better than others:

Revert #2037. This is the simplest solution, but we forgo its organizational benefits. On the other hand, this problem might be viewed as an argument that the tradeoffs aren't worth it.
Use TypeAlias, in an annotation, e.g. AutoInterrupt: TypeAlias = _AutoInterrupt. This is the obvious solution, and it seems to work fine. But the problem is that TypeAlias is only available in the standard library typing module since Python 3.10. Before that, it can be obtained from the typing_extensions package. But that package is currently only a dependency of GitPython when using Python 3.7.

I wouldn't hesitate even slightly to require typing_extensions for Python 3.8, which like 3.7 is end-of-life (no longer supported by the Python Software Foundation). People should avoid using EoL versions of Python, and should expect a somewhat degraded experience when doing so. But one of the long-standing benefits of GitPython is its minimal dependencies, so I'm reluctant to impose new ones on users of non-EoL versions without good reason.

However, typing_extensions is ubiquitous, so I wouldn't expect this to lead to problems for users/developers, nor for downstream packagers (operating systems that package GitPython surely package typing_extensions too). I think this is the way to go unless further complexities arise when doing it.
Actually make AutoInterrupt and CatFileContentStream top-level classes, and refer to them directly as such. For consistency with the usual expectations for conceptually public top-level classes of modules GitPython, we would probably also expose them in the top-level git module, by placing import statements and __all__ entires for them in __init__.py; but whether or not we did so, this is a change that would be non-breaking to implement but breaking to revert, if reverted after a release is made.

Abstractly speaking, the main problem here is that, unless it's combined with something like (2), it is an incomplete solution: the Git class will still need to contain aliases, and these will conceptually be aliases. In a sense, this even causes them to conceptually be aliases, in that their relationship to another type that is part of the public interface would be more than an implementation detail. So they should be treated as aliases, not variables, by the type system.

Concretely speaking, the main problem here is that code outside GitPython would still get the mypy errors, it would just have a way to make them go away. That's sometimes acceptable or even good--the point of static type checking is to find errors--but in this case the errors don't correspond to actual problems in the client code where they would arise. Therefore, I think that while it may be that this should be done eventually, it shouldn't be done as part of solving this bug.

(Also, do we intend that users/developers be able to customize the behavior of these classes by making a class that derives from Git that introduces new classes called AutoInterrupt and CatFileContentStream that inherit from Git.AutoInterrupt and Git.CatFileContentStream? If so, then we would have to be careful about where we use the module-level classes and where we use the Git member aliases, even though we intend them to be interchangeable.)
Change the top-level nonpublic _AutoInterrupt and _CatFileContentStream to _AutoInterruptBase and _CatFileContentStreamBase, define AutoInterrupt and CatFileContentStream as nested classes that inherit from them (and introduce derived __slots__ = () to avoid introducing __dict__s), and move the docstrings into the derived actually-nested classes.

This seems like it would be more confusing to work with when maintaining GitPython, and I am not confident that it will automatically preserve the intended effect with Sphinx, though it probably would. This is also arguably not a situation where the use of inheritance is warranted. But I can definitely look into this further if you think it might be preferable to (2).
Hacky ways to get mypy to recognize the classes as type aliases. For example, maybe placing them in another module and writing import statements for them inside the Git class would work! I think the badness of such approaches is self-evident, but in this case doing something like that would also have no clear benefit over (1), since it would place the classes' substantive definitions in an inconvenient and unintuitive place that would decrease readability.

If (2) is done, we can still revert both it and #2037 to get (1). So I'll do (2) unless further problems arise with it, with the idea that adding a typing_extensions dependency for more versions of Python, even one non-EoL version, may be acceptable.

Edit: This is done in #2039.

Activity

EliahKagan
added 2 commits that reference this issue on Jun 7, 2025
Start on fixing AutoInterrupt/CatFileContentStream aliases
c6d16d0
Import `TypeAlias` from `typing_extensions` where needed
c6c0812
EliahKagan
closed this as completedin #2039 on Jun 7, 2025

Metadata

Assignees

No one assigned

Labels

No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.