Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork937
Improve static typing and docstrings related to git object types#1859
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This revises the docstring of git.types.assert_never to moreclearly express its semantics and usage, to use the type names usedin the typing module, typing_extensions package, and mypy. Thisexpands some parts when doing so seemed to benefit clarity.The docstring had previously said AssertionError was raised, butthe code raised ValueError. For now I've adjusted the docstring tobe accurate to the code's behavior, but maybe this should change.I also removed the part about the mypy error being on variablecreation, since I am not clear on what that was referring to, andif it means the first name binding operation for a variable in itsscope, then I am not sure why that would be where an error messagewould be expected when using assert_never.Finally, this slightly adjusts the message:- "Literal" is decapitalized, to decrease confusion if the default message is used when matching on something not a typing.Literal.- The exception message prints the unexpected value's repr rather than its str, since the repr, when different from the str, is usually the representation more useful for debugging.
There is no advantage to fully qualifying names in the same moduleas the entity being documented, but I had accidentally done that.
This modifies the Tree docstring to replace the phrase "Tree as alist" with a phrase that:- Encompasses the use where subscripts are not integers or slices.- Is more precise in its terminology.The former advantage--not leaving out the case where one can passa string--is the rationale.
This drops the newly introduced precision in Python terminology,likening usage to that of a list or dict (experienced Python userswill already know about sequences and mappings, and inexperiencedusers will, with these terms, be able to understand what is said).Note that this does not undo the broader effect of the previouscommit, which is to make clear that subscripting a tree supportsboth list-like and dict-like usage, not just list-like usage.
The Commit_ish docstring may require substantive revision. Thefollow claims about the reasion for the design of Commit_ish shouldbe checked, and if false removed or fixed, and if true thenpossibly clarified or otherwise refined:- "often usable where a commit-ish is expected" (both as for whether it is a reasonable generalization and for whether it is actually part of the reason Commit_ish includes Blob and Tree)- "It is done for practical reasons including backward compatibility." (for whether that is why it was done is or kept)
This expands the docstrings of the Object base class, as well asthe four leaf subclasses that represent git objects (Blob, Commit,TagObject, and Tree) to clarify those classes' relationship to gitobjects in general and specific types of git objects, to eachother, and where applicable to types defined in git.types.This includes links to specific sections of the gitglossary(7)manpage (as hosted online), where directly applicable.
In git.types.- Remove commented-out import of SymbolicReference, which is not used anywhere, nor mentioned in commented-out code.- Add a docstring to _T to note that it is used within GitPython. (Otherwise it looks left over, as no code in git.types uses it.)
These are not necessarily what one means by the "scope" of aconfiguration variable, in part because of the nature of the subtledistinction between "user" and "global", and in part because ofother issues such as how setting a variable for a single commandwith "-c" has a scope which is not listed.This also brings the docstring somewhat more in line with how thesevalues are documented elsewhere in GitPython.
In some recently introduced or expanded docstrings, I had overusedphrases like "kind of git object" with the hope of avoidingconfusion with the meanings of "type" relevant to Python (i.e.,"class" or "static type"). But this made the relationship to git'sown notion of "object type" less clear than it could be, especiallyin docstrings that also included links to the gitglossary(7) entryfor "object type" (because those links' relevance was less clear).This dials back the use of "kind" to where I am more confident thatit is clarifying or at least not confusing.
The benefits are that putting imports before newly introducednames (other than names like __all__) is recommended by PEP-8, andthat git.types.PathLike is not equivalent to os.PathLike andintroducing it after all imports may help avoid obscuring this.
And the commented-out imports that had been solely to support it.
To make clear that code outside GitPython would not typicallybenefit from using anything in that module.Seegitpython-developers#1854 for context.
The _assertion_msg_format module attribute (global variable) ofgit.objects.base was formerly used in an assertion check that hassince been commented out but not completely removed.It may be that both it and the commented-out code that uses itshould simply be removed (they will be in the git history, afterall), but this change just brings them in line by also commentingout the variable.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This was referencedMar 4, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks so much for your help with this!
I made a few remarks, but the biggest question certainly is ifCommit_ish can be fixed instead of trying to justify it in the documentation. To me it seems possible, but I may be very wrong about that.
Thanks for sharing your thoughts.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This fixes a static typing error reported by mypy.Using a separate variable, as I had done originally, does not seemto be clearer than rebinding the parameter, and in this case thecode is simpler and can be checked by mypy without needing anotherexplicit type annotation to be added. This fixes one mypy error.This also adds a comment to make clearer why the mapping passed inis copied when modifications are needed, rather than mutated.
This fixes another static typing error reported by mypy.(The annotation could be made more specific in the future by makinga custom protocol for it, which may or may not be worthwhile, giventhat `**kwargs: Any` would still have to be present after whatevertyped keyword arguments the protocol's `__call__` method listed,since some callers intentionally forward arbitrary extra keywordarguments through safer_popen to Popen.)
On Windows, Python's subprocess module contains constants useful topass to the `creationflags` parameter of subprocess.Popen. Theseare absent on other platforms, where they are not meaningful.The code was already safe at runtime from any AttributeErrorrelated to these constants, because they were only used ingit.cmd._safer_popen_windows, which was defined on Windows.But ingitpython-developers#1792 I did not write the code in a way where mypy canverify its correctness. So if a regression of the kind mypy can inprinciple catch were to occur, it would be harder to notice it.This refactors the code, keeping the same behavior but expressingit in a way mypy can understand. This consists of two changes:1. Only define _safer_popen_windows when the platform is Windows, placing it in the first branch of the `if` statement. This is needed because mypy will not take the only current call to that nonpublic function being on Windows as sufficient evidence that the platform is always Windows when it is run.2. Determine the platform, for this purpose, using sys.platform instead of os.name. These are far from equivalent in general (see the deprecation rationale for is_<platform> ingitpython-developers#1732, revised ina0fa2bd ingitpython-developers#1787). However, in Python 3 (GitPython no longer supports Python 2), in the specific case of Windows, we have a choice of which to use, as both `sys.platform == "win32"` and `os.name == "nt"`. os.name is "nt" on native Windows, and "posix" on Cygwin. sys.platform is "win32" on native Windows (including 64-bit systems with 64-bit Python builds), and "cygwin" on Cygwin. See:https://docs.python.org/3/library/sys.html#sys.platform This is needed because the type stubs for the subprocess module use this sys.platform check (rather than an os.name check) to determine if the platform is Windows for the purpose of deciding which constants to say the subprocess module defines.I have verified that neither of these changes is enough by itself.
This changes how Git.execute defines the kill_process callback andhow it performs checks, fixing two mypy errors on Windows about howthe signal module doesn't have SIGKILL. In doing so, it alsoeliminates the need for the assertion added for safety and clarityin2f017ac (gitpython-developers#1761), since now kill_process is only defined if it isto be used (which is also guarded by a platform check, needed bymypy).As indc95a76 before this, part of the change here is to replacesome os.named-based checks with sys.platform-based checks, which issafe because, when one is specifically checking only for thedistinction between native Windows and all other systems, one canuse either approach. (Seedc95a76 for more details on that.)
- Change every bare `# type: ignore` into `# type: ignore[reason]`, where the specific reason is verified by mypy.- Use separate `# type: ignore[X]` and `# type: ignore[Y]` for different parts of statement where a single bare suppression was doing double duty, with each suppression only on the specific parts (splitting the statement into multiple lines for this).- Use the same suppression twice on very long statements where only specific parts need suppressions.This also formats all these comments with the same spacing (mostbut not all already were). This makes them easier to grep for.
The conditional imports of Literal from either typing ortyping_extensions have to be done with if-else on the Pythonversion rather than with try-except, or it is a static type error.This makes that change, checking sys.version_info.(See discussion ingitpython-developers#1861 andgitpython-developers#1862 for broader context andbackground on why this logic, before and after this change, isrepeated across multiple modules.)This also reorders/regroups imports for consistency in some places,especially where a new import of the sys module (for version_info)would otherwise exacerbate inconsistency.Since the merge commit0b99041, the number of mypy errors hadincreased from 5 to 10. This fixes all the new mypy errors, so thecount is back to 5.
This was referencedMar 14, 2024
MemberAuthor
EliahKagan commentedMar 14, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think this is ready. Rather than writing a lot here about the most non-obvious aspects of the change, I've opened three more issues to describe the problems they address,#1873,#1874, and#1875, linked and marked as fixed in the edited PR description. The core changes here remain the type annotation fixes throughout, reducing the number of mypy errors to 5, and of course; the fix for#1858, and directly and indirectly related annotations; and of course the originally scoped changes in adding docstrings in I've commented in#1858 (comment) on naming and why I chose the name |
Byron approved these changesMar 14, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What an impressive attention to detail, I can hardly imagine how it's possible to comb through this many files while keeping a keen eye out on complex types, and end up improving them.
Whether perfectly accurate or not, I believe this PR marks a huge leap towards a typing that makes sense and helps with usage.
Thank you!
Uh oh!
There was an error while loading.Please reload this page.
| :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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for mentioning this. It's great that this type includes what GitPython can handle though, as one day it might mean it can be taught to resolve tags as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I had avoided changing this since it is not inherently a wrong definition ofTree_ish, but actually the situation you are describing, where it can be broader, is already the case. Furthermore, bothCommit_ish andTree_ish ingit.types make a lot more sense if they both includeTagObject rather than one of them including it and the other not. I've opened#1878 to add it.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 14, 2024
This includes a brief description of the Submodule.__init__parent_commit parameter in its docstring, rather than onlyreferring to the set_parent_commit method, whose semantics differdue to conversation and validation, and which accepts more typesthan just Commit or None.The wording is based on wording in set_parent_commit, adjusted forthe difference in types, and set_parent_commit remains referencefor further details.This builds on1f03e7f (gitpython-developers#1859) in improving the situation describedingitpython-developers#1869.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 15, 2024
The Tree_ish union omitted TagObject, whose instances are onlysometimes tree-ish, and unlike Commit_ish beforegitpython-developers#1859, it is notinherently 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 ingitpython-developers#1859): as, itself,an alternative in the union used to annotate the rev parameter ofthe Repo.tree method (whose other alternatives are str and None).A TagObject may be passed, and if it points to a tree or committhen that will be resolved. Just to avoid a mypy error, code doingthat would (before this change) have to convert it to str first.That annotation should be improved, and the best way to do it is tokeep it written the same way but change the definition of Tree_ishin git.types to include TagObject. The reason is that doing soalleviates a major unintuitive aspect of the relationship betweenthe Commit_ish and Tree_ish types: Commit_ish was broader thaneverything commit-ish, while Tree_ish was narrower than everythingtree-ish.I had not considered making this change ingitpython-developers#1859 because I didn'twant to modify Tree_ish unnecessarily, and its definition was notinherently a bug. However, the change to Commit_ish is sufficientlylarge (though it only affects static typing) that a change toTree_ish to make them coherent and intuitive may be justified.This commit changes Tree_ish so that, in addition to its Commit andTree alternatives, it also includes TagObject. This also updatesand simplifies its docstring accordingly, bringing it in line withthat of Commit_ish which is already defined with the same kind ofbreadth, and further revises both docstrings to more explicitlyclarify when tags are tree-ish or commit-ish and when they are not.This does not change the separate nonpublic Treeish type defined ingit.index.base (and named with no underscore), which omitsTagObject but includes bytes and str, and which is used to annotateparameters of the IndexFile.from_tree and IndexFile.merge_treemethods. Changes there may be valuable, but the goal here is justto build narrowly ongitpython-developers#1859 to address a shortcoming of therevisions to git.types.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 15, 2024
The Tree_ish union omitted TagObject, whose instances are onlysometimes tree-ish, and unlike Commit_ish beforegitpython-developers#1859, it is notinherently 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 ingitpython-developers#1859): as, itself,an alternative in the union used to annotate the rev parameter ofthe Repo.tree method (whose other alternatives are str and None).A TagObject may be passed, and if it points to a tree or committhen that will be resolved. Just to avoid a mypy error, code doingthat would (before this change) have to convert it to str first.That annotation should be improved, and the best way to do it is tokeep it written the same way but change the definition of Tree_ishin git.types to include TagObject. The reason is that doing soalleviates a major unintuitive aspect of the relationship betweenthe Commit_ish and Tree_ish types: Commit_ish was broader thaneverything commit-ish, while Tree_ish was narrower than everythingtree-ish.I had not considered making this change ingitpython-developers#1859 because I didn'twant to modify Tree_ish unnecessarily, and its definition was notinherently a bug. However, the change to Commit_ish is sufficientlylarge (though it only affects static typing) that a change toTree_ish to make them coherent and intuitive may be justified.This commit changes Tree_ish so that, in addition to its Commit andTree alternatives, it also includes TagObject. This also updatesand simplifies its docstring accordingly, bringing it in line withthat of Commit_ish which is already defined with the same kind ofbreadth, and further revises both docstrings to more explicitlyclarify when tags are tree-ish or commit-ish and when they are not.This does not change the separate nonpublic Treeish type defined ingit.index.base (and named with no underscore), which omitsTagObject but includes bytes and str, and which is used to annotateparameters of the IndexFile.from_tree and IndexFile.merge_treemethods. Changes there may be valuable, but the goal here is justto build narrowly ongitpython-developers#1859 to address a shortcoming of therevisions to git.types.
Because of its effects on type checking of code that uses the GitPython library, this PR should probably be distinguished in some way, and as such may merit highlighting per#1806 (comment). If#1878 is accepted, then it contributes to this effect on type annotations, but so long as the effect is discernible I don't think they both have to be highlighted. This touches on some questions about the goals being achieved by marking pull requests in this way, but that is not specific to this so I'll ask about it in the#1806 comments instead. |
Thanks for pointing this out - I have marked it. Please do feel free to do so yourself in future, I trust your judgement and know I am likely to forget it |
Now that the purpose of the highlighting is clarified, per#1806 (comment), I'll go ahead and add it where I'm aware it's applicable. in future pull requests I open. |
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 18, 2024
The submodules being made explicit here are of course Pythonsubmodules, not git submodules.The git.objects.submodule Python submodule (which is *about* gitsubmodules) is made explicit here (as are the names imported fromthat Python submodule's own Python submodules) along with the otherPython submodules of git.objects.Unlike some other submodules, but like the top-level git moduleuntilgitpython-developers#1659, git.objects already defined __all__ but it wasdynamically constructed. As with git.__all__ previously (as notedingitpython-developers#1859), I have usedhttps://github.com/EliahKagan/deltall hereto check that it is safe, sufficient, and probably necessary toreplace this dynamically constructed __all__ with a literal list ofstrings in which all of the original elements are included. See:https://gist.github.com/EliahKagan/e627603717ca5f9cafaf8de9d9f27ad7Running the modattrs.py script, and diffing against the output frombefore the current import refactoring began, reveals two changes tomodule contents as a result of the change here:- git.objects no longer contains `inspect`, which it imported just to build the dynamically constructed __all__. Because this was not itself included in that __all__ or otherwise made public or used out of git.objects, that is okay. This is exactly analogous to the situation in8197e90, which removed it from the top-level git module aftergitpython-developers#1659.- The top-level git module now has has new attributes blob, commit, submodule, and tree, aliasing the corresponding modules. This has happened as a result of them being put in git.objects.__all__ along with various names imported from them. (As noted in some prior commits, there are tradeoffs associated with doing this, and it may be that such elements of __all__ should be removed, here and elsewhere.)
renovatebot referenced this pull request in allenporter/flux-localMar 31, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.42` -> `==3.1.43` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.43`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.43)[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.42...3.1.43)#### Particularly Important ChangesThese are likely to affect you, please do take a careful look.- Issue and test deprecation warnings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1886](https://togithub.com/gitpython-developers/GitPython/pull/1886)- Fix version_info cache invalidation, typing, parsing, andserialization by [@​EliahKagan](https://togithub.com/EliahKagan)in[https://github.com/gitpython-developers/GitPython/pull/1838](https://togithub.com/gitpython-developers/GitPython/pull/1838)- Document manual refresh path treatment by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1839](https://togithub.com/gitpython-developers/GitPython/pull/1839)- Improve static typing and docstrings related to git object types by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1859](https://togithub.com/gitpython-developers/GitPython/pull/1859)#### Other Changes- Test in Docker with Alpine Linux on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1826](https://togithub.com/gitpython-developers/GitPython/pull/1826)- Build online docs (RTD) with -W and dependencies by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1843](https://togithub.com/gitpython-developers/GitPython/pull/1843)- Suggest full-path refresh() in failure message by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1844](https://togithub.com/gitpython-developers/GitPython/pull/1844)- `repo.blame` and `repo.blame_incremental` now accept `None` as the`rev` parameter. by [@​Gaubbe](https://togithub.com/Gaubbe) in[https://github.com/gitpython-developers/GitPython/pull/1846](https://togithub.com/gitpython-developers/GitPython/pull/1846)- Make sure diff always uses the default diff driver when`create_patch=True` by[@​can-taslicukur](https://togithub.com/can-taslicukur) in[https://github.com/gitpython-developers/GitPython/pull/1832](https://togithub.com/gitpython-developers/GitPython/pull/1832)- Revise docstrings, comments, and a few messages by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1850](https://togithub.com/gitpython-developers/GitPython/pull/1850)- Expand what is included in the API Reference by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1855](https://togithub.com/gitpython-developers/GitPython/pull/1855)- Restore building of documentation downloads by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1856](https://togithub.com/gitpython-developers/GitPython/pull/1856)- Revise type annotations slightly by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1860](https://togithub.com/gitpython-developers/GitPython/pull/1860)- Updating regex pattern to handle unicode whitespaces. by[@​jcole-crowdstrike](https://togithub.com/jcole-crowdstrike) in[https://github.com/gitpython-developers/GitPython/pull/1853](https://togithub.com/gitpython-developers/GitPython/pull/1853)- Use upgraded pip in test fixture virtual environment by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1864](https://togithub.com/gitpython-developers/GitPython/pull/1864)- lint: replace `flake8` with `ruff` check by[@​Borda](https://togithub.com/Borda) in[https://github.com/gitpython-developers/GitPython/pull/1862](https://togithub.com/gitpython-developers/GitPython/pull/1862)- lint: switch Black with `ruff-format` by[@​Borda](https://togithub.com/Borda) in[https://github.com/gitpython-developers/GitPython/pull/1865](https://togithub.com/gitpython-developers/GitPython/pull/1865)- Update readme and tox.ini for recent tooling changes by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1868](https://togithub.com/gitpython-developers/GitPython/pull/1868)- Split tox lint env into three envs, all safe by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1870](https://togithub.com/gitpython-developers/GitPython/pull/1870)- Slightly broaden Ruff, and update and clarify tool configuration by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1871](https://togithub.com/gitpython-developers/GitPython/pull/1871)- Add a "doc" extra for documentation build dependencies by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1872](https://togithub.com/gitpython-developers/GitPython/pull/1872)- Describe `Submodule.__init__` parent_commit parameter by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1877](https://togithub.com/gitpython-developers/GitPython/pull/1877)- Include TagObject in git.types.Tree_ish by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1878](https://togithub.com/gitpython-developers/GitPython/pull/1878)- Improve Sphinx role usage, including linking Git manpages by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1879](https://togithub.com/gitpython-developers/GitPython/pull/1879)- Replace all wildcard imports with explicit imports by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1880](https://togithub.com/gitpython-developers/GitPython/pull/1880)- Clarify how tag objects are usually tree-ish and commit-ish by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1881](https://togithub.com/gitpython-developers/GitPython/pull/1881)#### New Contributors- [@​Gaubbe](https://togithub.com/Gaubbe) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1846](https://togithub.com/gitpython-developers/GitPython/pull/1846)- [@​can-taslicukur](https://togithub.com/can-taslicukur) madetheir first contribution in[https://github.com/gitpython-developers/GitPython/pull/1832](https://togithub.com/gitpython-developers/GitPython/pull/1832)- [@​jcole-crowdstrike](https://togithub.com/jcole-crowdstrike)made their first contribution in[https://github.com/gitpython-developers/GitPython/pull/1853](https://togithub.com/gitpython-developers/GitPython/pull/1853)- [@​Borda](https://togithub.com/Borda) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1862](https://togithub.com/gitpython-developers/GitPython/pull/1862)**Full Changelog**:gitpython-developers/GitPython@3.1.42...3.1.43</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Enabled.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/allenporter/flux-local).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
%5d(https%3a%2f%2frenovatebot.com){kind=link}
lettuce-botbot referenced this pull request in lettuce-financial/github-bot-signed-commitApr 1, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.42` -> `==3.1.43` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.43`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.43)[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.42...3.1.43)#### Particularly Important ChangesThese are likely to affect you, please do take a careful look.- Issue and test deprecation warnings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1886](https://togithub.com/gitpython-developers/GitPython/pull/1886)- Fix version_info cache invalidation, typing, parsing, andserialization by [@​EliahKagan](https://togithub.com/EliahKagan)in[https://github.com/gitpython-developers/GitPython/pull/1838](https://togithub.com/gitpython-developers/GitPython/pull/1838)- Document manual refresh path treatment by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1839](https://togithub.com/gitpython-developers/GitPython/pull/1839)- Improve static typing and docstrings related to git object types by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1859](https://togithub.com/gitpython-developers/GitPython/pull/1859)#### Other Changes- Test in Docker with Alpine Linux on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1826](https://togithub.com/gitpython-developers/GitPython/pull/1826)- Build online docs (RTD) with -W and dependencies by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1843](https://togithub.com/gitpython-developers/GitPython/pull/1843)- Suggest full-path refresh() in failure message by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1844](https://togithub.com/gitpython-developers/GitPython/pull/1844)- `repo.blame` and `repo.blame_incremental` now accept `None` as the`rev` parameter. by [@​Gaubbe](https://togithub.com/Gaubbe) in[https://github.com/gitpython-developers/GitPython/pull/1846](https://togithub.com/gitpython-developers/GitPython/pull/1846)- Make sure diff always uses the default diff driver when`create_patch=True` by[@​can-taslicukur](https://togithub.com/can-taslicukur) in[https://github.com/gitpython-developers/GitPython/pull/1832](https://togithub.com/gitpython-developers/GitPython/pull/1832)- Revise docstrings, comments, and a few messages by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1850](https://togithub.com/gitpython-developers/GitPython/pull/1850)- Expand what is included in the API Reference by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1855](https://togithub.com/gitpython-developers/GitPython/pull/1855)- Restore building of documentation downloads by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1856](https://togithub.com/gitpython-developers/GitPython/pull/1856)- Revise type annotations slightly by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1860](https://togithub.com/gitpython-developers/GitPython/pull/1860)- Updating regex pattern to handle unicode whitespaces. by[@​jcole-crowdstrike](https://togithub.com/jcole-crowdstrike) in[https://github.com/gitpython-developers/GitPython/pull/1853](https://togithub.com/gitpython-developers/GitPython/pull/1853)- Use upgraded pip in test fixture virtual environment by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1864](https://togithub.com/gitpython-developers/GitPython/pull/1864)- lint: replace `flake8` with `ruff` check by[@​Borda](https://togithub.com/Borda) in[https://github.com/gitpython-developers/GitPython/pull/1862](https://togithub.com/gitpython-developers/GitPython/pull/1862)- lint: switch Black with `ruff-format` by[@​Borda](https://togithub.com/Borda) in[https://github.com/gitpython-developers/GitPython/pull/1865](https://togithub.com/gitpython-developers/GitPython/pull/1865)- Update readme and tox.ini for recent tooling changes by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1868](https://togithub.com/gitpython-developers/GitPython/pull/1868)- Split tox lint env into three envs, all safe by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1870](https://togithub.com/gitpython-developers/GitPython/pull/1870)- Slightly broaden Ruff, and update and clarify tool configuration by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1871](https://togithub.com/gitpython-developers/GitPython/pull/1871)- Add a "doc" extra for documentation build dependencies by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1872](https://togithub.com/gitpython-developers/GitPython/pull/1872)- Describe `Submodule.__init__` parent_commit parameter by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1877](https://togithub.com/gitpython-developers/GitPython/pull/1877)- Include TagObject in git.types.Tree_ish by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1878](https://togithub.com/gitpython-developers/GitPython/pull/1878)- Improve Sphinx role usage, including linking Git manpages by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1879](https://togithub.com/gitpython-developers/GitPython/pull/1879)- Replace all wildcard imports with explicit imports by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1880](https://togithub.com/gitpython-developers/GitPython/pull/1880)- Clarify how tag objects are usually tree-ish and commit-ish by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1881](https://togithub.com/gitpython-developers/GitPython/pull/1881)#### New Contributors- [@​Gaubbe](https://togithub.com/Gaubbe) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1846](https://togithub.com/gitpython-developers/GitPython/pull/1846)- [@​can-taslicukur](https://togithub.com/can-taslicukur) madetheir first contribution in[https://github.com/gitpython-developers/GitPython/pull/1832](https://togithub.com/gitpython-developers/GitPython/pull/1832)- [@​jcole-crowdstrike](https://togithub.com/jcole-crowdstrike)made their first contribution in[https://github.com/gitpython-developers/GitPython/pull/1853](https://togithub.com/gitpython-developers/GitPython/pull/1853)- [@​Borda](https://togithub.com/Borda) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1862](https://togithub.com/gitpython-developers/GitPython/pull/1862)**Full Changelog**:gitpython-developers/GitPython@3.1.42...3.1.43</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Disabled by config. Please merge this manually once youare satisfied.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about theseupdates again.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/lettuce-financial/github-bot-signed-commit).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
📣highlight-in-changelog📣Specifically highlight this PR in the changelog after it was created
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
Fixes#1857
Fixes#1858
Fixes#1866
Fixes#1873
Fixes#1874
Fixes#1875
Note: This pull request expanded considerably in scope since first opened, which its title reflects but the rest of this description does not. See comments for details.
Main changes
This expands the docstrings of
Objectand some of its subclasses. These revisions are, in large part, to explain the classes' relationships to the git concept of objects and to the corresponding git object types, including links to relevant definitions ingitglossary(7).This also adds docstrings to all the types introduced in
git.types--those that relate directly to git object types and those that do not. Adding documentation ingit.typesrelates topically to the docstring expansions forObjectand some of its subclasses, and I tried to make all new and edited docstrings make sense whether read by themselves or together with other docstrings across modules. It also relates to the goal forgit.typesdocumentation articulated in#1854 (comment).Other changes
This also includes some other changes that are less core to that scope but that seem practically useful to include. It revises the docstring for
assert_never, which had been the only docstring ingit.types, and modifies the string of the default exception it raises to print thereprrather than thestr. It removes some long-unused nonpublic names at module level, and revises some method docstrings of classes whose class docstrings were being revised. It also expands thegit.compatmodule docstring, toward the goal for it that was also articulated in#1854 (comment).Considerations for
assert_neverThe change to
assert_never, which includes making the docstring accurately describe the current (and established) behavior of the function, mayfix#1857. However, as described there I am unsure if this is ultimately the right change there.Considerations for
Commit_ishdocstring (including possible inaccuracies)Even more important is the relationship of the
Commit_ishdocstring to#1858. I have said this PR "may" fix that issue, but I strongly suspect that either further refinement will be needed or that a significantly greater change, possibly outside the scope of this PR, may be needed.Furthermore, some of the text in that docstring may require improvement before being suitable for merging. To facilitate writing the docstrings in
git.types, including related docstrings and even other parts of theCommit_ishdocstring, I wrote text in two parts of theCommit_ishdocstringthat sound plausible but that I am not at all sure are correct.This is in regard to the question, detailed in#1858, ofwhy
Commit_ishunion includes the never-commitishTreeandBlobclasses. (Even if what I wrote turns out to be correct, it may need to be clarified.) I have opened review comments on those potentially inaccurate lines.