Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 8, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Always use a def instead of assigning a lambda
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.
  • Loading branch information
EliahKagan committed Jun 8, 2025
commit a36b8a5a726ee5a17cfd492e49c0b0b2a05b2136
19 changes: 15 additions & 4 deletions git/index/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,10 @@ def unmerged_blobs(self) -> Dict[PathLike, List[Tuple[StageType, Blob]]]:
stage. That is, a file removed on the 'other' branch whose entries are at
stage 3 will not have a stage 3 entry.
"""
is_unmerged_blob = lambda t: t[0] != 0

def is_unmerged_blob(t: Tuple[StageType, Blob]) -> bool:
return t[0] != 0

path_map: Dict[PathLike, List[Tuple[StageType, Blob]]] = {}
for stage, blob in self.iter_blobs(is_unmerged_blob):
path_map.setdefault(blob.path, []).append((stage, blob))
Expand Down Expand Up @@ -690,12 +693,17 @@ def _store_path(self, filepath: PathLike, fprogress: Callable) -> BaseIndexEntry
This must be ensured in the calling code.
"""
st = os.lstat(filepath) # Handles non-symlinks as well.

if S_ISLNK(st.st_mode):
# In PY3, readlink is a string, but we need bytes.
# In PY2, it was just OS encoded bytes, we assumed UTF-8.
open_stream: Callable[[], BinaryIO] = lambda: BytesIO(force_bytes(os.readlink(filepath), encoding=defenc))
def open_stream() -> BinaryIO:
return BytesIO(force_bytes(os.readlink(filepath), encoding=defenc))
else:
open_stream = lambda: open(filepath, "rb")

def open_stream() -> BinaryIO:
return open(filepath, "rb")

with open_stream() as stream:
fprogress(filepath, False, filepath)
istream = self.repo.odb.store(IStream(Blob.type, st.st_size, stream))
Expand Down Expand Up @@ -1336,8 +1344,11 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik
kwargs["as_process"] = True
kwargs["istream"] = subprocess.PIPE
proc = self.repo.git.checkout_index(args, **kwargs)

# FIXME: Reading from GIL!
make_exc = lambda: GitCommandError(("git-checkout-index",) + tuple(args), 128, proc.stderr.read())
def make_exc() -> GitCommandError:
return GitCommandError(("git-checkout-index", *args), 128, proc.stderr.read())

checked_out_files: List[PathLike] = []

for path in paths:
Expand Down
4 changes: 3 additions & 1 deletion git/objects/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@

# --------------------------------------------------------

cmp: Callable[[str, str], int] = lambda a, b: (a > b) - (a < b)

def cmp(a: str, b: str) -> int:
return (a > b) - (a < b)


class TreeModifier:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ lint.extend-select = [
"TC004", # See: https://docs.astral.sh/ruff/rules/runtime-import-in-type-checking-block/
]
lint.ignore = [
"E731", # Do not assign a `lambda` expression, use a `def`
# If it becomes necessary to ignore any rules, list them here.
]
lint.unfixable = [
"F401", # Module imported but unused
Expand Down
6 changes: 5 additions & 1 deletion test/test_fun.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ def test_tree_traversal(self):
B_old = self.rorepo.tree("1f66cfbbce58b4b552b041707a12d437cc5f400a") # old base tree

# Two very different trees.

entries = traverse_trees_recursive(odb, [B_old.binsha, H.binsha], "")
self._assert_tree_entries(entries, 2)

Expand All @@ -251,7 +252,10 @@ def test_tree_traversal(self):
self._assert_tree_entries(oentries, 2)

# Single tree.
is_no_tree = lambda i, d: i.type != "tree"

def is_no_tree(i, _d):
return i.type != "tree"

entries = traverse_trees_recursive(odb, [B.binsha], "")
assert len(entries) == len(list(B.traverse(predicate=is_no_tree)))
self._assert_tree_entries(entries, 1)
Expand Down
5 changes: 4 additions & 1 deletion test/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,10 @@ def test_index_file_from_tree(self, rw_repo):
assert len([e for e in three_way_index.entries.values() if e.stage != 0])

# ITERATE BLOBS
merge_required = lambda t: t[0] != 0

def merge_required(t):
return t[0] != 0

merge_blobs = list(three_way_index.iter_blobs(merge_required))
assert merge_blobs
assert merge_blobs[0][0] in (1, 2, 3)
Expand Down
10 changes: 8 additions & 2 deletions test/test_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,18 @@ def test_traverse(self):
assert len(list(root)) == len(list(root.traverse(depth=1)))

# Only choose trees.
trees_only = lambda i, d: i.type == "tree"

def trees_only(i, _d):
return i.type == "tree"

trees = list(root.traverse(predicate=trees_only))
assert len(trees) == len([i for i in root.traverse() if trees_only(i, 0)])

# Test prune.
lib_folder = lambda t, d: t.path == "lib"

def lib_folder(t, _d):
return t.path == "lib"

pruned_trees = list(root.traverse(predicate=trees_only, prune=lib_folder))
assert len(pruned_trees) < len(trees)

Expand Down
Loading