René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Git.{AutoInterrupt,CatFileContentStream} refactor broke static typing","articleBody":"#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):\n\n```diff\ndiff --git a/a b/b\nindex 128f3547..59913c9c 100644\n--- a/a\n+++ b/b\n@@ -1,33 +1,82 @@\n git\\types.py:133: error: Type[...] can't contain \"Union[Literal[...], Literal[...]]\"  [valid-type]\n+git\\util.py:508: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\util.py:508: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\util.py:512: error: Item Git.AutoInterrupt? of \"Union[Popen[Any], Git.AutoInterrupt?]\" has no attribute \"wait\"  [union-attr]\n git\\util.py:1146: error: Variance of TypeVar \"T_IterableObj\" incompatible with variance in parent type  [type-var]\n git\\util.py:1217: error: Argument 2 to \"getattr\" has incompatible type \"Union[SupportsIndex, str]\"; expected \"str\"  [arg-type]\n git\\util.py:1219: error: Unsupported operand types for + (\"str\" and \"SupportsIndex\")  [operator]\n git\\util.py:1219: note: Right operand is of type \"Union[SupportsIndex, str]\"\n git\\util.py:1226: error: Unsupported operand types for + (\"str\" and \"SupportsIndex\")  [operator]\n git\\util.py:1226: error: Unsupported operand types for + (\"str\" and \"slice[Any, Any, Any]\")  [operator]\n git\\util.py:1226: note: Right operand is of type \"Union[SupportsIndex, slice[Any, Any, Any], str]\"\n 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]\n git\\config.py:628: error: Redundant cast to \"Union[str, PathLike[str]]\"  [redundant-cast]\n+git\\cmd.py:98: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:98: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:106: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:106: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:170: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:170: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:218: error: Unused \"type: ignore\" comment  [unused-ignore]\n+git\\cmd.py:510: error: Variable \"git.cmd.Git.CatFileContentStream\" is not valid as a type  [valid-type]\n+git\\cmd.py:510: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1051: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1051: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1108: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1108: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1528: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1528: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1533: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1533: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1537: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1537: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1658: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1658: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1668: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1668: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1671: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\cmd.py:1671: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1672: error: Git.AutoInterrupt? has no attribute \"stdin\"  [attr-defined]\n+git\\cmd.py:1672: error: Git.AutoInterrupt? has no attribute \"stdout\"  [attr-defined]\n+git\\cmd.py:1673: error: Git.AutoInterrupt? has no attribute \"stdin\"  [attr-defined]\n+git\\cmd.py:1674: error: Git.AutoInterrupt? has no attribute \"stdin\"  [attr-defined]\n+git\\cmd.py:1675: error: Git.AutoInterrupt? has no attribute \"stdout\"  [attr-defined]\n+git\\cmd.py:1703: error: Git.CatFileContentStream? has no attribute \"read\"  [attr-defined]\n+git\\cmd.py:1707: error: Variable \"git.cmd.Git.CatFileContentStream\" is not valid as a type  [valid-type]\n+git\\cmd.py:1707: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\cmd.py:1719: error: Git.AutoInterrupt? has no attribute \"stdout\"  [attr-defined]\n 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]\n+git\\diff.py:584: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\diff.py:584: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n git\\objects\\submodule\\base.py:834: error: \"SymbolicReference\" has no attribute \"set_tracking_branch\"  [attr-defined]\n git\\objects\\submodule\\base.py:862: error: \"SymbolicReference\" has no attribute \"tracking_branch\"  [attr-defined]\n+git\\objects\\commit.py:454: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\objects\\commit.py:454: note: Error code \"valid-type\" not covered by \"type: ignore\" comment\n+git\\objects\\commit.py:454: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\objects\\commit.py:459: error: Git.AutoInterrupt? has no attribute \"communicate\"  [attr-defined]\n git\\objects\\commit.py:711: error: Argument 2 to \"create\" of \"SymbolicReference\" has incompatible type \"Callable[[], SymbolicReference]\"; expected \"Union[str, PathLike[str]]\"  [arg-type]\n git\\objects\\commit.py:903: error: Argument 2 to \"findall\" has incompatible type \"Union[str, bytes]\"; expected \"str\"  [arg-type]\n git\\refs\\symbolic.py:919: error: Incompatible types in assignment (expression has type \"SymbolicReference\", variable has type \"T_References\")  [assignment]\n git\\refs\\tag.py:48: error: Read-only property cannot override read-write property  [misc]\n git\\refs\\tag.py:49: error: Unused \"type: ignore\" comment  [unused-ignore]\n git\\refs\\tag.py:83: error: Read-only property cannot override read-write property  [misc]\n git\\refs\\tag.py:84: error: Unused \"type: ignore\" comment  [unused-ignore]\n git\\refs\\head.py:48: error: Incompatible override of a setter type  [override]\n git\\refs\\head.py:48: note:  (base class \"SymbolicReference\" defined the type as \"Union[Commit, SymbolicReference, str]\",\n git\\refs\\head.py:48: note:  override has type \"Commit\")\n git\\refs\\head.py:48: note:  Setter types should behave contravariantly\n+git\\remote.py:874: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\remote.py:874: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\remote.py:902: error: Git.AutoInterrupt? has no attribute \"wait\"  [attr-defined]\n+git\\remote.py:945: error: Variable \"git.cmd.Git.AutoInterrupt\" is not valid as a type  [valid-type]\n+git\\remote.py:945: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases\n+git\\remote.py:975: error: Git.AutoInterrupt? has no attribute \"wait\"  [attr-defined]\n git\\repo\\fun.py:289: error: \"Callable[[], SymbolicReference]\" has no attribute \"commit\"  [attr-defined]\n git\\repo\\fun.py:339: error: \"Callable[[], SymbolicReference]\" has no attribute \"log_entry\"  [attr-defined]\n git\\repo\\base.py:688: error: Redundant cast to \"Literal['system', 'global', 'user', 'repository']\"  [redundant-cast]\n git\\repo\\base.py:690: error: Redundant cast to \"Literal['system', 'global', 'user', 'repository']\"  [redundant-cast]\n git\\repo\\base.py:1054: error: Incompatible return value type (got \"SymbolicReference\", expected \"Head\")  [return-value]\n 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]\n 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]\n 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]\n-Found 27 errors in 12 files (checked 44 source files)\n+Found 57 errors in 15 files (checked 44 source files)\n```\n\nThe problem is that, as described in [Variables vs type aliases](https://mypy.readthedocs.io/en/stable/common_issues.html#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).\n\nSo 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).\n\n**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.\n\nI'm aware of several possible solutions, some better than others:\n\n1. Revert [#2037](https://github.com/gitpython-developers/GitPython/pull/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.\n\n2. 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.\n\n   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.\n\n   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.\n\n3. 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.\n\n   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.\n\n   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.\n\n   (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.)\n\n4. 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.\n\n   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).\n\n5. 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.\n\nIf (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.\n\n*Edit:* This is done in #2039.","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2025-06-07T15:32:45.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/2038/GitPython/issues/2038"}