Skip to content

Include TagObject in git.types.Tree_ish #1878

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

Merged
merged 1 commit into from
Mar 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Include TagObject in git.types.Tree_ish
The Tree_ish union omitted TagObject, whose instances are only
sometimes tree-ish, and unlike Commit_ish before #1859, it is not
inherently a bug to define Tree_ish this way.

However, this Tree_ish type actually has only one use in GitPython
(which was also the case before the changes in #1859): as, itself,
an alternative in the union used to annotate the rev parameter of
the Repo.tree method (whose other alternatives are str and None).
A TagObject may be passed, and if it points to a tree or commit
then that will be resolved. Just to avoid a mypy error, code doing
that would (before this change) have to convert it to str first.

That annotation should be improved, and the best way to do it is to
keep it written the same way but change the definition of Tree_ish
in git.types to include TagObject. The reason is that doing so
alleviates a major unintuitive aspect of the relationship between
the Commit_ish and Tree_ish types: Commit_ish was broader than
everything commit-ish, while Tree_ish was narrower than everything
tree-ish.

I had not considered making this change in #1859 because I didn't
want to modify Tree_ish unnecessarily, and its definition was not
inherently a bug. However, the change to Commit_ish is sufficiently
large (though it only affects static typing) that a change to
Tree_ish to make them coherent and intuitive may be justified.

This commit changes Tree_ish so that, in addition to its Commit and
Tree alternatives, it also includes TagObject. This also updates
and simplifies its docstring accordingly, bringing it in line with
that of Commit_ish which is already defined with the same kind of
breadth, and further revises both docstrings to more explicitly
clarify when tags are tree-ish or commit-ish and when they are not.

This does not change the separate nonpublic Treeish type defined in
git.index.base (and named with no underscore), which omits
TagObject but includes bytes and str, and which is used to annotate
parameters of the IndexFile.from_tree and IndexFile.merge_tree
methods. Changes there may be valuable, but the goal here is just
to build narrowly on #1859 to address a shortcoming of the
revisions to git.types.
  • Loading branch information
EliahKagan committed Mar 15, 2024
commit 70ef69ae118ef8f393eb3f5c84e72e24df09516b
25 changes: 10 additions & 15 deletions git/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,18 @@
See also the :class:`Tree_ish` and :class:`Commit_ish` unions.
"""

Tree_ish = Union["Commit", "Tree"]
"""Union of :class:`~git.objects.base.Object`-based types that are inherently tree-ish.
Tree_ish = Union["Commit", "Tree", "TagObject"]
"""Union of :class:`~git.objects.base.Object`-based types that are sometimes tree-ish.

See gitglossary(7) on "tree-ish": https://git-scm.com/docs/gitglossary#def_tree-ish

:note:
This union comprises **only** the :class:`~git.objects.commit.Commit` and
:class:`~git.objects.tree.Tree` classes, **all** of whose instances are tree-ish.
This has been done because of the way GitPython uses it as a static type annotation.

:class:`~git.objects.tag.TagObject`, some but not all of whose instances are
tree-ish (those representing git tag objects that ultimately resolve to a tree or
commit), is not covered as part of this union type.
:class:`~git.objects.tree.Tree` and :class:`~git.objects.commit.Commit` are the
classes whose instances are all tree-ish. This union includes them, but also
:class:`~git.objects.tag.TagObject`, only **some** of whose instances are tree-ish.
Whether a particular :class:`~git.objects.tag.TagObject` peels (recursively
dereferences) to a tree or commit, rather than a blob, can in general only be known
at runtime.

:note:
See also the :class:`AnyGitObject` union of all four classes corresponding to git
Expand All @@ -102,12 +101,8 @@
commit-ish. This union type includes :class:`~git.objects.commit.Commit`, but also
:class:`~git.objects.tag.TagObject`, only **some** of whose instances are
commit-ish. Whether a particular :class:`~git.objects.tag.TagObject` peels
(recursively dereferences) to a commit can in general only be known at runtime.

:note:
This is an inversion of the situation with :class:`Tree_ish`. This union is broader
than all commit-ish objects, while :class:`Tree_ish` is narrower than all tree-ish
objects.
(recursively dereferences) to a commit, rather than a tree or blob, can in general
only be known at runtime.

:note:
See also the :class:`AnyGitObject` union of all four classes corresponding to git
Expand Down