Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork941
Various style improvements#2049
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
Merged
+48 −24
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
`requirements-dev.txt`, but none of the others, was tracked withWindows-style (CRLF) line endings. This appears to have been thecase since it was introduced ina1b7634 (as `dev-requirements.txt`)and not to be intentional.This only changes how it is stored in the repository. This does notchange `.gitattributes` (it is not forced to have LF line endingsif automatic line-ending conversions are configured in Git).
This resolves two warnings about Ruff configuration, by:- No longer setting `ignore-init-module-imports = true` explicitly, which was deprecated since `ruff` 0.4.4. We primarily use `ruff` via `pre-commit`, for which this deprecation has applied since we upgraded the version in `.pre-commit-config.yaml` from 0.4.3 to 0.6.0 ind1582d1 (gitpython-developers#1953). We continue to list `F401` ("Module imported but unused") as not automatically fixable, to avoid inadvertently removing imports that may be needed. See also:https://docs.astral.sh/ruff/settings/#lint_ignore-init-module-imports- Rename the rule `TCH004` to `TC004`, since `TCH004` is the old name that may eventually be removed and that is deprecated since 0.8.0. We upgraded `ruff` in `.pre-commit-config.yml` again inb7ce712 (gitpython-developers#2031), from 0.6.0 to 0.11.12, at which point this deprecation applied. See alsohttps://astral.sh/blog/ruff-v0.8.0.These changes make those configuration-related warnings go away,and no new diagnostics (errors/warnings) are produced when running`ruff check` or `pre-commit run --all-files`. No F401-relateddiagnostics are triggered when testing with explicit`ignore-init-module-imports = false`, in preview mode or otherwise.In addition, this commit makes two changes that are not needed toresolve warnings:- Stop excluding `E203` ("Whitespace before ':'"). That diagnostic is no longer failing with the current code here in the current version of `ruff`, and code changes that would cause it to fail would likely be accidentally mis-st- Add the version lower bound `>=0.8` for `ruff` in `requirements-dev.txt`. That file is rarely used, as noted ina8a73ff (gitpython-developers#1871), but as long as we have it, there may be a benefit to excluding dependency versions for which our configuration is no longer compatible. This is the only change in this commit outside of `pyproject.toml`.
This stops listing Ruff rule `E731` ("Do not assign a `lambda`expression, use a `def`") as ignored, and fixes all occurrences ofit:- Spacing is manually adjusted so that readability is not harmed, while still satisfying the current formatting conventions.- Although the affected test modules do not currently use type annotations, the non-test modules do. Some of the lambdas already had type annotations, by annotating the variable itself with an expression formed by subscripting `Callable`. This change preserves them, converting them to paramter and return type annotations in the resulting `def`. Where such type annotations were absent (in lambdas in non-test modules), or partly absent, all missing annotations are added to the `def`.- Unused paramters are prefixed with a `_`.- `IndexFile.checkout` assigned a lambda to `make_exc`, whose body was somewhat difficult to read. Separately from converting it to a `def`, this refactors the expression in the `return` statement to use code like `(x, *ys)` in place of `(x,) + tuple(ys)`.This change does not appear to have introduced (nor fixed) any`mypy` errors.This only affects lambdas that were assigned directly to variables.Other lambda expressions remain unchanged.
17d53b0
intogitpython-developers:main 27 checks passed
Uh oh!
There was an error while loading.Please reload this page.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
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.
This makes various style improvements:
requirements-dev.txt
.ruff
configuration warnings (about the configuration itself) and enables a rule that was turned off explicitly but that we are now following.def f
overf = lambda
, changes the code to follow that rule, and makes associated changes to the lambda to make the use of newlines more readable, adds parameter and return type annotations where missing or incomplete, and refactors the expression of them returns for clarity.Full details are in the commit messages.