Comments (6)
Does it mean that PR is welcome? =)
from gitpython.
Let's wait for @EliahKagan to chime in on this, he has been doing a lot of work recently and has a plan. I say Rust tools are great, but I am biased ;).
from gitpython.
Thanks!
CC @EliahKagan , who has been contemplating with the idea as well.
from gitpython.
Yes, I think this is a great time to switch to Ruff.
The main problem I encountered last time I worked on this is that Ruff didn't understand the special semantics of typing.Literal
when it was imported indirectly from GitPython's git.types
module. This caused a valuable check not to be usable, due to too many false positives. (Unfortunately I don't remember the details about the check itself. I can look this up and will try to do so, but I wanted to reply now rather than delaying further.)
Literal
is different from most types that define __class_getitem__
. This is because, for most classes X
that support subscripts, X["Y"]
means something like X[Y]
, allowing Y
to be referred even if declared afterwards or in an import guarded by if TYPE_CHECKING:
. In contrast, Literal["Y"]
does not use the type Y
as the value of a generic type argument, but instead is a type whose one and only value is the literal string "Y"
.
Special rules are required to distinguish between them. Static type checkers such as mypy
and pyright
recognize GitPython's usage where Literal
is conditionally imported in git.types
either from typing
or typing_extensions
depending on the Python version. At least as of when I last tried this with Ruff, it did not recognize this. So it thought that subexpressions like "Y"
in type annotations like Literal["Y"]
referred to a type like "Y"
rather than a value, and complained that no such type was defined.
My recollection is that this particular check is one that seems quite valuable for GitPython to have, and that flake8 is currently managing to provide it, without the false positives Ruff gives (or at least, at that time, gave). I am reluctant to disable it. Suppression comments could maybe be used, if they can be made narrow enough. I vaguely (and perhaps wrongly) recall that this seemed unwieldy to do at the time.
An obvious possible solution is for GitPython to drop support for Python 3.7, since typing
has Literal
since 3.8. After all, Python 3.7 has been end-of-life for quite some time now. Then the imports could be changed to take Literal
directly from typing
. Although I think GitPython should drop 3.7 support fairly soon--and maybe even immediately if a significant benefit from a 3.8+ feature arises for GitPython's code or its unit tests--it seems to me that tooling changes are not a strong reason to drop 3.7 right now, especially since Ruff still officially supports Python 3.7 even in its latest version.
I am, however, biased in this area, because a feature branch I'm working on (that is related to symlinks on Windows and does not yet have a PR, and which I hope to pick back up after #1859) benefits from the continued inclusion of 3.7, in that 3.7 and later versions of Python differ in their handling of symlinks in a way that reveals weaknesses in the approach I am taking that would still be weaknesses even in the absence of a need to support 3.7. This is to say that, ironically, it benefits from 3.7 because it is hard to support 3.7. I acknowledge that "We should keep 3.7 support for a while longer because it is causing problems" is not a very strong argument. πΈ
In spite of this, the reason I say now is a good time to switch to Ruff is that a long-standing flake8-related limitation has come up, relating to its interaction with black. The version of black that will automatically be installed with pip install -e '.[test]'
is now newer than the version specified in .pre_commit_config,.yml
and used by pre-commit
. The new version of black will format type stubs with the ...
body on the same line as the :
that precedes it (when there is enough space), rather than on a separate line as most function bodies are written. This is the more widely accepted style and it makes sense that black now prefers it. However, this conflicts with flake8's rules. The relevant rule is actually disabled by default, but some projects, including GitPython, enable it (I don't remember if GitPython does this explicitly or through a use of exclude
rather than extend-exclude
). GitPython doesn't carry any .pyi
files, but @overload
annotated stubs that precede an actual implementation qualify as type stubs.
Unlike the rule at stake with Literal
, this is a fairly unimportant rule, since probably no style checker is needed to avoid writing non-stub functions on one line, and the bad impact of doing so by accident would also be pretty small. So that rule could just be turned off, and the versions for flake8 and its plugins in .pre-commit-config.yml
could be updated. And maybe Ruff will even have this problem, too. Yet I still think that makes this a good time to try again in switching to Ruff; the linting setup has to be revisited now-ish anyway, after all.
Because Ruff at least previously had an issue with GitPython's use of Literal
from git.types
, and #1859 makes heavy changes to the git.types
module, as well as changes to numerous type annotations some of which use Literal
, it might be easier to wait until that PR is done. But I don't want to keep you waiting in case you want to move faster. (Likewise, I'll try to look up the mentioned rules and, where applicable, relevant GitHub issues, but I didn't want that research to be a blocker here.)
from gitpython.
An obvious possible solution is for GitPython to drop support for Python 3.7, since
typing
hasLiteral
since 3.8.
in different projects, we used to use from typing_extensions import Literal
from gitpython.
Yes, that may be the way to go.
Depending how it is done, it could require adding typing_extensions
as a dependency of GitPython
even for Python 3.8 and later. Currently that dependency is for python_version<"3.8"
. It seems to me that adding it for all versions is not currently justified for this (though it may be justified in the future for forthcoming features, such as PEP 702).
However, when a type checker is installed and used, I believe typing_extensions
is available. So if the typing_extensions
imports are guarded by if TYPE_CHECKING:
then I think it should work even without changing GitPython's main dependencies. (It is fine if typing_extensions
is a direct or indirect dependency of an extra; my concern is about the main dependencies.)
This may be a little cumbersome for Literal
with string literal subscripts if the whole thing is quoted, but most of those in GitPython are defined as type aliases and the alias name used. (from __future__ import annotations
will eliminate the need for the outer quotes, of course. Some GitPython modules use this and most don't; I'm not sure if there's any guideline in GitPython when it comes to that.)
from gitpython.
Related Issues (20)
- The API Reference documents too few modules HOT 2
- assert_never raises different exception than documented HOT 1
- Commit_ish is much broader than commit-ish HOT 4
- origin.pull(rebase=True) sometimes lost local commit , gitpyton version 3.1.31 HOT 3
- Wrong type annotation on `Index.commit` for `parent_commits` HOT 3
- Using create submodule doesn't create the correct submodule structure HOT 4
- Submodule.__init__ `parent_commit` conversion/validation is implied but not done HOT 8
- Module docstring says TagReference but means TagObject
- Diffable.diff is misleadingly annotated regarding special `other` values HOT 3
- Top-level refresh function not listed in __all__ HOT 1
- Bad git executable HOT 2
- Unable to specify lines of context while doing diff (-U flag) HOT 1
- γremote().exist()γ exception when lack remote section in config file
- Unhandled IndexError when calling .read() on a malformed config file HOT 6
- Detailed xfail output sometimes distracts from unexpected failures
- Some xfail markings fail to validate their exception types HOT 1
- How can i set create_patch=true, and meanwhile get change_type in Diff
- git.Repo.clone_from() not working correctly in WSL HOT 3
- Fuzz Tests Are Crashing at Start-up on ClusterFuzz HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gitpython.