Various style improvements #2049
Merged
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 with Windows-style (CRLF) line endings. This appears to have been the case since it was introduced in a1b7634 (as `dev-requirements.txt`) and not to be intentional. This only changes how it is stored in the repository. This does not change `.gitattributes` (it is not forced to have LF line endings if 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 in d1582d1 (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 in b7ce712 (gitpython-developers#2031), from 0.6.0 to 0.11.12, at which point this deprecation applied. See also https://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-related diagnostics 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 to resolve 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 in a8a73ff (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 of
it:
- 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.
Sign up for free
to 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.
This makes various style improvements:
requirements-dev.txt.ruffconfiguration warnings (about the configuration itself) and enables a rule that was turned off explicitly but that we are now following.def foverf = 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.