Include TagObject in git.types.Tree_ish

[EliahKagan]

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 pull request 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.

Notes

This is related to #1859 (comment).

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.

[Byron]

Thanks a lot!

I am glad this change could be made as it means that GitPython can properly peel input objects.

As a nit, I felt that saying TagObject only sometimes resolves to a tree doesn't do justice to their typical use, which is to point to a commit. However, I am also sure you had a good reason to describe it like this.

[EliahKagan]

As a nit, I felt that saying TagObject only sometimes resolves to a tree doesn't do justice to their typical use, which is to point to a commit. However, I am also sure you had a good reason to describe it like this.

That sounds like something that should be further improved. But where is it?

[Byron]

I left comments where I thought the nit comes from, apologies for being unspecific previously.

[EliahKagan]

Do you just mean that this change should be made to Tree_ish, changing "tree or commit" to "commit or tree"?

 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
-dereferences) to a commit or tree, rather than a blob, can in general only be known
 at runtime. 

If so, then the following elaborated request for clarification can be disregarded.

---

If not--and it sounds to me like you may intend to suggest a deeper change--then I think I am still missing something. The relevant pieces are:

GitPython/git/types.py

Lines 86 to 89 in 0a609b9

	: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.

GitPython/git/types.py

Lines 105 to 108 in 0a609b9

	: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, rather than a tree or blob, can in general
	only be known at runtime.

These are saying only some tag objects are tree-ish and only some tag objects are commit-ish, respectively. This seems different to me from saying that tag objects only sometimes resolve to a tree; when tag objects are tree-ish, it is usually because they resolve to a commit, which is tree-ish.

I can think of three changes that might help.

• "Only some" could be changed to "Only most" or, perhaps better, "Only some (most)".
• Further explanation can be added regarding what makes something commit-ish or tree-ish.
• Adding "making it tree-ish" and "making it commit-ish" before "rather than a".

However, it seems to me that perhaps none of these is likely what you're looking for either, because it sounds like you may have identified an unaddressed asymmetry, while these changes apply equally to both.

[Byron]

Sorry for not making it clearer before. My 'nit' was about TagObjects being referred to as 'sometimes' pointing to a commit, even though they do that most of the time. To my mind it's already great to show they are resolved at runtime to indicate they could also point to another object type.

[EliahKagan]

Thanks for clarifying, that makes sense now. I'll try to open a pull request to make that aspect, about what is most common, more clear, sometime later today.

[EliahKagan]

I've opened #1881 for this.