[pre-commit] update ruff to v0.16 - #1967
Merged
Merged
Conversation
Contributor
|
Thanks @tdavidcl for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
📝 WalkthroughWalkthroughThe PR applies Ruff 0.16.0 and updates Python formatting, iteration, imports, exception types, raw strings, and small logic expressions across buildbot, environment, example, library, and tool modules. ChangesPython modernization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@buildbot/comp_soudwave.py`:
- Around line 192-193: Update the loop in the dictionary-processing block to
iterate directly over dic_tmp rather than dic_tmp.keys(), and replace the
membership condition with the idiomatic not in form while preserving the
existing body behavior.
In `@buildbot/configure.py`:
- Line 99: Resolve all Ruff TRY003 diagnostics for the string-based raises in
buildbot/configure.py at lines 99-99, 102-102, 224-226, 290-292, and 321-321,
and buildbot/lib/buildbot.py at lines 186-186, 251-251, and 260-260. Define and
use appropriate project-specific exception types consistently, or add an
approved Ruff exclusion covering every affected raise.
In `@buildbot/convert_to_vtk.py`:
- Around line 275-276: Replace explicit dictionary key iteration and negated
membership checks at buildbot/convert_to_vtk.py lines 275-276 with direct
iteration and `not in`; update buildbot/lib/make_report.py line 106 to use
direct negative membership; and update buildbot/plot_profdata.py lines 25 and 41
similarly, preserving the existing control flow.
In `@buildbot/make_status_file_table.py`:
- Line 77: In the file-reading loop, rename the iterator variable in the `for l
in f` statement to `line` and update every reference to `l` within that loop
body accordingly.
In `@buildbot/plot_profdata.py`:
- Line 46: Update the labels assignment to use list(dic_labels) directly instead
of the redundant comprehension, preserving the same label ordering and values.
In `@buildbot/precommit_report.py`:
- Around line 51-57: Replace the chained filename comparisons in the allowlist
conditional with collection membership using a set, preserving all five existing
filenames and the surrounding behavior.
In `@buildbot/print_compile_stats.py`:
- Line 16: Rename the ambiguous loop variable from l to line and update all
references in buildbot/print_compile_stats.py lines 16-16,
buildbot/tmp_plot_patches.py lines 63-63, and buildbot/tmp_plot_patches.py lines
100-100; no other behavior should change.
In `@buildbot/tmp_plot_patches.py`:
- Line 98: Update the file-opening logic around fil and the subsequent parsing
loop to use a with open(a, "r") as fil context manager, keeping the loop inside
the block so the file closes reliably even when parsing raises an exception.
In `@env/utils/amd_arch.py`:
- Around line 12-18: Apply the architecture-idiom cleanup in all three sites: in
env/utils/amd_arch.py lines 12-18, build AMD_ARCH_LIST with list(AMD_ARCH_DESC)
and use arch_code not in AMD_ARCH_LIST within print_description; in
env/utils/cuda_arch.py lines 20-26, build NVIDIA_ARCH_LIST with
list(NVIDIA_ARCH_DESC) and use arch_code not in NVIDIA_ARCH_LIST; in
env/utils/intel_llvm.py lines 63-67, replace the target membership check with
args.target not in arch_list.
In `@examples/physics/run_coala.py`:
- Around line 70-72: Update the merged kernel condition in the surrounding
branch to use membership testing, replacing the `kernel == 2 or kernel == 3`
expression with an equivalent `kernel in {2, 3}` check while preserving the
existing assignments.
In `@examples/physics/run_fmm.py`:
- Line 1101: Define an UnsupportedTensorCollectionTypeError subclass of
TypeError with the formatted unsupported-type message in its initializer, then
update the raise site in the tensor collection handling flow to raise
UnsupportedTensorCollectionTypeError(type(d)) instead of constructing the
formatted message inline.
In `@examples/sph/run_dustysettle_tva.py`:
- Around line 511-515: Replace the if/else assignment to trap_func with a
conditional expression that selects np.trapezoid when available and np.trapz
otherwise, preserving the existing NPY201 suppression on the fallback reference.
In `@examples/sph/run_show_all_sph_kernels.py`:
- Around line 30-33: Update the plot_test_sph_kernel function parameters to
address the unused f and df arguments: rename them to _f and _df if the callback
signature is not externally required; otherwise retain the names and add a
targeted noqa: ARG001 with a reason.
In `@examples/sph/run_sph_taylor_green_vortex.py`:
- Line 188: Add a targeted `FBT003` noqa directive to the `model.do_vtk_dump`
call while keeping the boolean argument positional. Do not convert `True` to a
keyword argument or broaden the suppression beyond this binding call.
In `@examples/TO_MIGRATE/visualization/animate_sedov_csv.py`:
- Line 167: Rename the comprehension variable from l to line in both snapshot
loaders: examples/TO_MIGRATE/visualization/animate_sedov_csv.py lines 167-167
and examples/TO_MIGRATE/visualization/animate_sod_csv.py lines 88-88, preserving
the existing filtering behavior.
In `@tools/check_pragma_once.py`:
- Line 38: In the pragma-detection logic around the visible startswith
condition, replace the nested else/if branch with an elif branch, preserving the
existing conditions and behavior while eliminating the PLR5501 warning.
In `@tools/make_version_file.py`:
- Line 11: Replace the broad exception-swallowing try/except around the
version-file read in the version-file generation flow with an explicit
FileNotFoundError handler. Preserve the missing-file fallback, while logging or
re-raising other read failures instead of silently ignoring them.
In `@tools/update_authors.py`:
- Around line 173-174: In the autocorrect loop over splt, rename the ambiguous l
variable to a descriptive name and update its references, then replace the
negated membership expression with the direct not-in form while preserving the
existing condition behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 74f4e505-059f-4c78-92b5-b3dd959315a2
📒 Files selected for processing (88)
.pre-commit-config.yamlbuildbot/analyse_include_stats.pybuildbot/clang_tidy_diff_report.pybuildbot/comp_soudwave.pybuildbot/configure.pybuildbot/convert_to_vtk.pybuildbot/doxygen_diff.pybuildbot/generate_callgraph.pybuildbot/lib/buildbot.pybuildbot/lib/make_report.pybuildbot/make_status_file_table.pybuildbot/plot_profdata.pybuildbot/precommit_report.pybuildbot/print_compile_stats.pybuildbot/test_pipeline_old.pybuildbot/tmp_plot_output.pybuildbot/tmp_plot_patches.pydoc/mkdocs/docs/assets/figures/scalling_tests_sph_sedov.pyenv/helpers/_pysetup.pyenv/machine/cbp/generic/intel-llvm/setup-env.pyenv/machine/dgx-cbp/intel-llvm/setup-env.pyenv/machine/lumi/g/acpp-custom-llvm/setup-env.pyenv/new-envenv/utils/amd_arch.pyenv/utils/cuda_arch.pyenv/utils/intel_llvm.pyenv/utils/sysinfo.pyexamples/TO_MIGRATE/benchmarks/sedov_scale_test_updated.pyexamples/TO_MIGRATE/misc/plot_test.pyexamples/TO_MIGRATE/sph/sph_disc.pyexamples/TO_MIGRATE/sph/sph_soundwave.pyexamples/TO_MIGRATE/visualization/animate_sedov_csv.pyexamples/TO_MIGRATE/visualization/animate_sod_csv.pyexamples/benchmarks/run_compute_histogram.pyexamples/benchmarks/run_dtt_performance.pyexamples/benchmarks/run_is_all_true_performance.pyexamples/benchmarks/run_reduction_performance.pyexamples/benchmarks/run_segmented_sort_in_place_performance.pyexamples/benchmarks/sph_homogeneous_benchmark.pyexamples/benchmarks/sph_weak_scale_test.pyexamples/physics/run_coala.pyexamples/physics/run_dustywave_sympy.pyexamples/physics/run_fmm.pyexamples/ramses/run_kh.pyexamples/ramses/run_linear_wave_with_bc.pyexamples/ramses/run_toro_shocks.pyexamples/sph/run_advect_sphere_domain_decomp.pyexamples/sph/run_circular_disc_lense_thirring.pyexamples/sph/run_cubic_reorganisation.pyexamples/sph/run_dustydiffuse_tva.pyexamples/sph/run_dustysettle_tva.pyexamples/sph/run_dustywave_tva.pyexamples/sph/run_init_sim_from_other.py.pyexamples/sph/run_kelvin_helmholtz.pyexamples/sph/run_kernels_sympy.pyexamples/sph/run_kill_particle_sphere.pyexamples/sph/run_pairing_instab.pyexamples/sph/run_show_all_sph_kernels.pyexamples/sph/run_sod.pyexamples/sph/run_sod_dust_tva.pyexamples/sph/run_sph_basic_disc.pyexamples/sph/run_sph_custom_warp_profile.pyexamples/sph/run_sph_shear_test.pyexamples/sph/run_sph_taylor_green_vortex.pyexamples/sph/run_sphsetup_logs.pyexamples/sph/run_start_sph_from_phantom_dump.pyexamples/sph/run_uniform_box.pyexamples/sph/run_upscale_simulation_restart.pyexamples/tests_ci/regression_godunov_soundwave_3d.pyexamples/tests_ci/regression_sph_disc.pyexamples/tests_ci/run_compare_shamrock_ph_disc.pyexamples/tests_ci/run_sg_compare_error_sph.pyexamples/tests_ci/sod_tube_zeus.pysrc/pylib/shamrock/__init__.pysrc/pylib/shamrock/external/coala/interface_coala_shamrock.pysrc/pylib/shamrock/external/coala/iterate_coag.pysrc/pylib/shamrock/utils/SimulationRunner.pysrc/pylib/shamrock/utils/analysis/StandardPlotHelper.pysrc/pylib/shamrock/utils/analysis/UnitHelper.pysrc/pylib/shamrock/utils/analysis/VelocityPlots.pysrc/pylib/shamrock/utils/analysis/__init__.pysrc/pylib/shamrock/utils/dump/__init__.pysrc/pylib/shamrock/utils/plot/__init__.pysrc/shambindings/src/run_ipython.pytools/check_pragma_once.pytools/make_version_file.pytools/shamrock_tool_banner.pytools/update_authors.py
Comment on lines
192
to
+193
| for k in dic_tmp.keys(): | ||
| if not k in dic.keys(): | ||
| if not k in dic: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Complete the direct-iteration rewrite.
dic_tmp.keys() is unnecessary, and not k in dic still triggers E713. Use direct iteration and not in.
Proposed fix
- for k in dic_tmp.keys():
- if not k in dic:
+ for k in dic_tmp:
+ if k not in dic:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for k in dic_tmp.keys(): | |
| if not k in dic.keys(): | |
| if not k in dic: | |
| for k in dic_tmp: | |
| if k not in dic: |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 192-192: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
[error] 193-193: Test for membership should be not in
Convert to not in
(E713)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@buildbot/comp_soudwave.py` around lines 192 - 193, Update the loop in the
dictionary-processing block to iterate directly over dic_tmp rather than
dic_tmp.keys(), and replace the membership condition with the idiomatic not in
form while preserving the existing body behavior.
Source: Linters/SAST tools
|
|
||
| if not (args.profile == None): | ||
| raise "can not select a profile with a unknown compiler" | ||
| raise ValueError("can not select a profile with a unknown compiler") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(configure\.py|buildbot\.py|pyproject\.toml|ruff\.toml|\.ruff\.toml)$' || true
echo "== configure.py relevant lines =="
if [ -f buildbot/configure.py ]; then
nl -ba buildbot/configure.py | sed -n '70,340p'
fi
echo "== buildbot.py relevant lines =="
if [ -f buildbot/lib/buildbot.py ]; then
nl -ba buildbot/lib/buildbot.py | sed -n '160,270p'
fi
echo "== project-specific exceptions and ruff config references =="
rg -n "class .*Exception|Exception|TRY003|select|ignore|per-file-ignores|extend-per-file-ignores" buildbot pyproject.toml ruff.toml .ruff.toml 2>/dev/null || trueRepository: Shamrock-code/Shamrock
Length of output: 293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral/source verifier: show all raise ValueError expressions in the reviewed files
# and whether any project-defined exception class is used as target of raises nearby.
python3 - <<'PY'
from pathlib import Path
import ast
targets = [Path("buildbot/configure.py"), Path("buildbot/lib/buildbot.py")]
for path in targets:
if not path.exists():
continue
tree = ast.parse(path.read_text(), filename=str(path))
exception_classes = set()
for cls in [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]:
names = []
for base in cls.bases:
name = getattr(base, "id", None)
if name:
names.append(name)
if names:
exception_classes.add((path, cls.name, names))
print(f"\n== {path} exception classes ==")
print("\n".join(f"{c[0]}:{c[1]} bases={c[2]}" for c in exception_classes))
print(f"\n== {path} raises ==")
for node in ast.walk(tree):
if isinstance(node, ast.Raise):
expr = node.exc
if expr:
text = ast.unparse(expr) if hasattr(ast, "unparse") else None
print(f"{path}:{node.lineno} raise {text or '...'}")
PYRepository: Shamrock-code/Shamrock
Length of output: 905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant configure.py lines =="
awk '{printf "%6d\t%s\n", NR, $0}' buildbot/configure.py | sed -n '85,110p;218,298p;315,325p'
echo "== relevant buildbot.py lines =="
awk '{printf "%6d\t%s\n", NR, $0}' buildbot/lib/buildbot.py | sed -n '178,268p'
echo "== project exception classes and Ruff config/check references =="
awk '
/^class .*Exception[:(]/{class=$0; getline line; if(line ~ /Exception[):]/ || line ~ /RuntimeError[):]/) class=class line; print class; status=1; next}
status && /^[[:space:]]+pass|^[[:space:]]+#|^[[:space:]]+\"\"\"|^[[:space:]]/ { status=0 }
status && !/^[[:space:]]*$/ {print}
/' buildbot/configure.py buildbot/lib/buildbot.py
rg -n "class .*Exception|try:|except|TRY003|ruff|per-file-ignores|extend-per-file-ignores|select|ignore" buildbot pyproject.toml ruff.toml .ruff.toml 2>/dev/null || true
echo "== focused raise and exception name AST verifier =="
python3 - <<'PY'
from pathlib import Path
import ast
for path in [Path('buildbot/configure.py'), Path('buildbot/lib/buildbot.py')]:
if not path.exists():
continue
tree = ast.parse(path.read_text(), filename=str(path))
print(f"\n== {path} exception classes ==")
exception_classes = []
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef):
bases = []
for base in node.bases:
if isinstance(base, ast.Name):
bases.append((base.lineno, base.id))
if any(b[1] in {"Exception", "RuntimeError", "ValueError"} for b in bases):
exception_classes.append((node.lineno, node.name, bases))
print("\n".join(f"{p[0]}:{p[1]} bases={p[2]}" for p in exception_classes))
print(f"\n== {path} raises ==")
for node in ast.walk(tree):
if isinstance(node, ast.Raise):
expr = node.exc
if isinstance(expr, ast.Call) and isinstance(expr.func, ast.Name) and expr.func.id == "ValueError" and expr.args:
print(f"{path}:{node.lineno} raise ValueError({expr.args[0].value!r})")
PYRepository: Shamrock-code/Shamrock
Length of output: 8838
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exception definitions and Ruff references =="
for f in $(git ls-files 'buildbot/**/*.py' pyproject.toml ruff.toml .ruff.toml); do
case "$f" in
*py/*pyproject.toml|ruff.toml|.ruff.toml)
echo "--- $f"
rg -n "class .*Exception|Exception|RuntimeError|ValueError|try:|except|TRY003|ruff|per-file-ignores|extend-per-file-ignores|select|ignore" "$f" || true
;;
esac
done
echo "== all ValueError raise lines in reviewed files =="
python3 - <<'PY'
from pathlib import Path
import ast
for path in [Path('buildbot/configure.py'), Path('buildbot/lib/buildbot.py')]:
if not path.exists():
continue
print(f"\n== {path} ==")
tree = ast.parse(path.read_text(), filename=str(path))
print("exception classes inheriting Exception/RuntimeError:")
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef):
for base in node.bases:
if isinstance(base, ast.Name) and base.id in {"Exception", "RuntimeError", "ValueError"}:
print(f"f'{path}:{node.lineno}:class {node.name}')
break
print("raise ValueError lines:")
for node in ast.walk(tree):
if isinstance(node, ast.Raise) and isinstance(node.exc, ast.Call) and isinstance(node.exc.func, ast.Name) and node.exc.func.id == "ValueError":
value = node.exc.args[0]
if isinstance(value, ast.Constant):
msg = value.value
elif isinstance(value, ast.JoinedStr):
msg = ast.unparse(value)
else:
msg = ast.unparse(value) if hasattr(ast, "unparse") else "<dynamic>"
print(f"f'{path}:{node.lineno}:raise ValueError({msg!r})")
PYRepository: Shamrock-code/Shamrock
Length of output: 443
Resolve the remaining Ruff TRY003 diagnostics.
Define project-specific exception types for these string raises, or apply an approved Ruff exclusion consistently.
buildbot/configure.py#L99,#L102,#L224,#L290,#L321buildbot/lib/buildbot.py#L186,#L251,#L260
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 99-99: Avoid specifying long messages outside the exception class
(TRY003)
📍 Affects 2 files
buildbot/configure.py#L99-L99(this comment)buildbot/configure.py#L102-L102buildbot/configure.py#L224-L226buildbot/configure.py#L290-L292buildbot/configure.py#L321-L321buildbot/lib/buildbot.py#L186-L186buildbot/lib/buildbot.py#L251-L251buildbot/lib/buildbot.py#L260-L260
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@buildbot/configure.py` at line 99, Resolve all Ruff TRY003 diagnostics for
the string-based raises in buildbot/configure.py at lines 99-99, 102-102,
224-226, 290-292, and 321-321, and buildbot/lib/buildbot.py at lines 186-186,
251-251, and 260-260. Define and use appropriate project-specific exception
types consistently, or add an approved Ruff exclusion covering every affected
raise.
Source: Linters/SAST tools
Comment on lines
275
to
+276
| for k in dic_tmp.keys(): | ||
| if not k in dic.keys(): | ||
| if not k in dic: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Python package / ruff availability:"
python3 - <<'PY'
import sys, importlib.util
print("python", sys.version.split()[0])
print("ruff", shutil.which("ruff") if "shutil" in globals() else None)
if importlib.util.find_spec("ruff"):
print("ruff installed as Python package")
else:
print("ruff not installed as Python package")
PY
echo
echo "Repository files:"
git ls-files | rg '(^|/)buildbot/(convert_to_vtk\.py|plot_profdata\.py|lib/make_report\.py)$|(^|/)pyproject\.toml$|(^|/)ruff\.toml$|(^|/)setup\.cfg$' || true
echo
echo "Relevant snippets:"
for f in buildbot/convert_to_vtk.py buildbot/plot_profdata.py buildbot/lib/make_report.py; do
if [ -f "$f" ]; then
echo "--- $f line count"
wc -l "$f"
echo "--- $f context"
nl -ba "$f" | sed -n '20,55p;100,110p;268,282p'
fi
done
echo
echo "Search for ruff diagnostics / config:"
rg -n "UP017|UP025|in_not_in|SIM|Q003|lint|select|ignore|preview" -g 'pyproject.toml' -g 'ruff*.toml' -g 'setup.cfg' . || trueRepository: Shamrock-code/Shamrock
Length of output: 530
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Deterministic semantic probe for the exact forms under review.
# Demonstrate that `not (key in mapping)` and `key not in mapping`
# produce the same boolean result for the same `key`/mapping objects.
dic = {"a": 1, "x": 2}
dic_tmp = {"a": 3, "z": 4}
dic_labels = {"x": 10}
cur_test = "z"
entry = {"name": "z"}
cases = {
"convert_to_vtk_keys": (dic_tmp.keys(), dic, "k"),
"convert_to_vtk_missing": (dic_tmp.keys(), dic, "k"),
"make_report": ("z", {"x": True}, "cur_test"),
"plot_profdata_name": (entry["name"], {"a": True}, None),
"plot_profdata_labels": ("z", dic_labels, "k"),
}
for label, (term, mapping, name) in cases.items():
value = term if name is None else {"k": term}[name]
old = not (value in mapping)
new = value not in mapping
print(label, old, new, old == new)
# Iteration comparison: .keys() and direct dict iteration produce the same
# key sequence for deterministic mapping state.
same = list(dic_tmp.keys()) == list(dic_tmp)
print("dict direct iteration equals .keys():", same)
PY
echo
echo "Static read-only check for exact expressions:"
python3 - <<'PY'
import ast, pathlib, re
files = [
pathlib.Path("buildbot/convert_to_vtk.py"),
pathlib.Path("buildbot/plot_profdata.py"),
pathlib.Path("buildbot/lib/make_report.py"),
]
for path in files:
if not path.exists():
continue
text = path.read_text()
lines = text.splitlines()
hits = []
for i, line in enumerate(lines, 1):
# Match syntactic forms like `not (...) in (...)` / `not ( ... in mapping)`
# or `not k in dic`, but not `not k in dic`.
stripped = line.strip()
if re.search(r'\bnot\s*\([^)]*in\s+[^)]*\)', stripped) or re.search(r'\bnot\s+\w+\s+in\s+\w+', stripped):
hits.append((i, stripped))
print(path)
for i, line in hits:
print(f"{i}: {line}")
PYRepository: Shamrock-code/Shamrock
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
export LC_ALL=C
echo "Repository files:"
git ls-files | grep -E '(\bnamedtuple\.py|(^|/)buildbot/(convert_to_vtk\.py|plot_profdata\.py|lib/make_report\.py)$|(^|/)pyproject\.toml$|(^|/)ruff\.toml$|(^|/)setup\.cfg$)' || true
echo
echo "Python availability of read-only verifier:"
python3 - <<'PY'
import ast, pathlib, re
paths = [
pathlib.Path("buildbot/convert_to_vtk.py"),
pathlib.Path("buildbot/plot_profdata.py"),
pathlib.Path("buildbot/lib/make_report.py"),
]
for path in paths:
print(f"--- {path} exists={path.exists()} size={path.stat().st_size if path.exists() else 'n/a'}")
if not path.exists():
continue
text = path.read_text()
lines = text.splitlines()
print(f"--- {path} line count={len(lines)}")
print(f"--- {path} snippets:")
ranges = [("buildbot/convert_to_vtk.py", 268, 282), ("buildbot/plot_profdata.py", 18, 46), ("buildbot/lib/make_report.py", 100, 110)]
hits = []
for i, line in enumerate(lines, 1):
stripped = line.strip()
if re.search(r"\bnot\s*\(.*?\bin\b.*?\)", stripped) or re.search(r"\bnot\s+(\S+)\s+in\s+(\S+)", stripped):
hits.append((i, stripped))
for r in ranges:
fname, start, end = r
if path.samefile(fname if fname.startswith(pathlib.Path(".")) else fname):
for i in range(start, end + 1):
print(f"{i}: {lines[i-1]}")
print(f"--- {path} exact matches:")
for i, line in hits:
print(f"{i}: {line}")
print("--- code shape probe:")
tree = ast.parse(open("buildbot/convert_to_vtk.py", encoding="utf-8").read().splitlines()[273])
for node in ast.walk(tree):
if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not):
print(type(node.operand).__name__, ast.dump(node.operand, include_attributes=False))
PYRepository: Shamrock-code/Shamrock
Length of output: 594
🏁 Script executed:
#!/bin/bash
set -euo pipefail
export LC_ALL=C
python3 - <<'PY'
import ast, pathlib, re
paths = [
pathlib.Path("buildbot/convert_to_vtk.py"),
pathlib.Path("buildbot/plot_profdata.py"),
pathlib.Path("buildbot/lib/make_report.py"),
]
for path in paths:
print(f"--- {path} exists={path.exists()} size={path.stat().st_size if path.exists() else 'n/a'}")
if not path.exists():
continue
text = path.read_text()
lines = text.splitlines()
print(f"--- {path} line count={len(lines)}")
for start, end in {
"buildbot/convert_to_vtk.py": (268, 282),
"buildbot/plot_profdata.py": (18, 46),
"buildbot/lib/make_report.py": (100, 110),
}.get(str(path), []):
print(f"--- {path} lines {start}-{end}:")
for i in range(start, end + 1):
print(f"{i}: {lines[i-1]}")
vals = []
hits = []
for i, line in enumerate(lines, 1):
stripped = line.strip()
if re.search(r"\bnot\s*\(.*?\bin\b.*?\)", stripped) or re.search(r"\bnot\s+(\S+)\s+in\s+(\S+)", stripped):
hits.append((i, stripped))
try:
node = ast.parse(stripped, "<test>").body[0].value
except SyntaxError:
continue
if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not):
if isinstance(node.operand, ast.Compare):
cmp = node.operand
if cmp.ops and isinstance(cmp.ops[0], ast.In) and len(cmp.comparators) == 1:
vals.append(i)
print(f"--- {path} not_in hits={len(hits)} values={len(vals)}")
for i, line in hits:
print(f"{i}: {line}")
print(f"--- {path} .keys() iteration occurrences:")
for i, line in enumerate(lines, 1):
if re.search(r"\bnot\s*\(.*?\b\.keys\(\)\b.*?\bin\b.*?\)", line) or re.search(r"\bnot\s+\w+\.keys\(\)\s+in\b", line):
print(f"{i}: {line}")
print("--- dict_keys semantics probe:")
dic_tmp = {"a": 3, "z": 4}
# Use an actual dict so `in dic_tmp` is valid.
print(all(k not in {"a": 1} for k in dic_tmp.keys()))
print(all(k not in {"a": 1} for k in dic_tmp))
PY
echo
echo "Relevant config existence:"
git ls-files | grep -E '(^|/)(pyproject\.toml|ruff\.toml|setup\.cfg)$' || trueRepository: Shamrock-code/Shamrock
Length of output: 382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
export LC_ALL=C
python3 - <<'PY'
import ast, pathlib, re
paths = [
pathlib.Path("buildbot/convert_to_vtk.py"),
pathlib.Path("buildbot/plot_profdata.py"),
pathlib.Path("buildbot/lib/make_report.py"),
]
ranges = {
"buildbot/convert_to_vtk.py": (268, 282),
"buildbot/plot_profdata.py": (18, 46),
"buildbot/lib/make_report.py": (100, 110),
}
for path in paths:
print(f"--- {path} exists={path.exists().name}")
if not path.exists():
continue
text = path.read_text()
lines = text.splitlines()
print(f"--- {path} line count={len(lines)}")
base = str(path)
start, end = ranges[base]
print(f"--- {path} lines {start}-{end}:")
for i in range(start, end + 1):
print(f"{i}: {lines[i-1]}")
vals = []
hits = []
for i, line in enumerate(lines, 1):
stripped = line.strip()
if re.search(r"\bnot\s*\(.*?\bin\b.*?\)", stripped) or re.search(r"\bnot\s+(\S+)\s+in\s+(\S+)", stripped):
hits.append((i, stripped))
try:
node = ast.parse(stripped, "<test>").body[0].value
except SyntaxError:
continue
if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not):
operand = node.operand
if isinstance(operand, ast.Compare):
cmp = operand
if cmp.ops and isinstance(cmp.ops[0], ast.In) and len(cmp.comparators) == 1:
vals.append(i)
# Inspect left expression to handle direct .keys() iteration.
print(f"--- AST probe {path}:{i}: left={ast.dump(cmp.left, include_attributes=False)}")
print f"--- {path} not_in hits={len(hits)} in-comparisons={len(vals)}"
for i, line in hits:
print(f"{i}: {line}")
print("--- dict_keys semantics probe:")
dic_tmp = {"a": 3, "z": 4}
print("all k not in dict via .keys():", all(k not in {"a": 1} for k in dic_tmp.keys()))
print("all k not in dict via direct iteration:", all(k not in {"a": 1} for k in dic_tmp))
PY
echo
echo "Relevant config files:"
git ls-files | grep -E '(^|/)(pyproject\.toml|ruff\.toml|setup\.cfg)$' || trueRepository: Shamrock-code/Shamrock
Length of output: 301
Use direct dictionary syntax for the remaining Ruff diagnostics.
Replace not (key in mapping) with key not in mapping. Iterate dictionaries directly when only keys are required.
buildbot/convert_to_vtk.py#L275-L276: usefor k in dic_tmp:andif k not in dic:.buildbot/lib/make_report.py#L106-L106: useif cur_test not in dic_loaded:.buildbot/plot_profdata.py#L25-L25: useif entry["name"] not in dic:.buildbot/plot_profdata.py#L41-L41: useif k not in dic_labels:.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 275-275: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
[error] 276-276: Test for membership should be not in
Convert to not in
(E713)
📍 Affects 3 files
buildbot/convert_to_vtk.py#L275-L276(this comment)buildbot/lib/make_report.py#L106-L106buildbot/plot_profdata.py#L25-L25buildbot/plot_profdata.py#L41-L41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@buildbot/convert_to_vtk.py` around lines 275 - 276, Replace explicit
dictionary key iteration and negated membership checks at
buildbot/convert_to_vtk.py lines 275-276 with direct iteration and `not in`;
update buildbot/lib/make_report.py line 106 to use direct negative membership;
and update buildbot/plot_profdata.py lines 25 and 41 similarly, preserving the
existing control flow.
Source: Linters/SAST tools
| flag = "//%Impl status : " | ||
|
|
||
| for l in f.readlines(): | ||
| for l in f: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)make_status_file_table\.py$|(^|/)ruff|pyproject\.toml$' || true
echo
echo "Target file excerpt:"
file="$(git ls-files | rg '(^|/)make_status_file_table\.py$' | head -n1)"
if [ -n "${file:-}" ]; then
nl -ba "$file" | sed -n '60,90p'
fi
echo
echo "Rust/callers mentioning make_status_file_table:"
rg -n "make_status_file_table|implstatus|for l in f|startswith\(flag\)" . -g '!**/.git/**' || true
echo
echo "Python lint config occurrences:"
for f in pyproject.toml setup.cfg tox.ini ruff.toml .ruff.toml; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n "E741|flake8|lint|select|ignore|ruff" "$f" || true
fi
doneRepository: Shamrock-code/Shamrock
Length of output: 265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target file excerpt:"
python3 - <<'PY'
from pathlib import Path
p = Path("buildbot/make_status_file_table.py")
print(f"exists={p.exists()} size={p.stat().st_size}")
text = p.read_text()
lines = text.splitlines()
for i in range(60, min(95, len(lines))+1):
print(f"{i:4}: {lines[i-1]}")
PY
echo
echo "Relevant usages:"
rg -n "make_status_file_table|implstatus|for l in f|startswith\(flag\)" . || true
echo
echo "Python lint config occurrences:"
for f in pyproject.toml setup.cfg tox.ini ruff.toml .ruff.toml; do
if [ -f "$f" ]; then
echo "--- $f"
grep -Eni "E741|flake8|lint|select|ignore|ruff" "$f" || true
fi
done
echo
echo "Read-only parser check for variable `l` scope and assignments:"
python3 - <<'PY'
import ast
from pathlib import Path
tree = ast.parse(Path("buildbot/make_status_file_table.py").read_text())
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for t in node.targets:
if isinstance(t, ast.Name) and t.id == "l":
print("assignment:", t.lineno)
if isinstance(node, ast.AugAssign) and isinstance(node.target, ast.Name) and node.target.id == "l":
print("aug assignment:", node.lineno)
if isinstance(node, ast.For) and isinstance(node.target, ast.Name) and node.target.id == "l":
print("for binding:", node.lineno, col=node.col_offset)
print("done")
PYRepository: Shamrock-code/Shamrock
Length of output: 3460
Rename the loop variable l.
l can be confused with 1 or i; use line here and update the loop references.
🧰 Tools
🪛 Ruff (0.16.0)
[error] 77-77: Ambiguous variable name: l
(E741)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@buildbot/make_status_file_table.py` at line 77, In the file-reading loop,
rename the iterator variable in the `for l in f` statement to `line` and update
every reference to `l` within that loop body accordingly.
Source: Linters/SAST tools
|
|
||
| # label list for the plot | ||
| labels = [k for k in dic_labels.keys()] | ||
| labels = [k for k in dic_labels] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use list(dic_labels) directly.
The list comprehension has no transformation and triggers Ruff C416. Replace it with labels = list(dic_labels).
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 46-46: Unnecessary list comprehension (rewrite using list())
Rewrite using list()
(C416)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@buildbot/plot_profdata.py` at line 46, Update the labels assignment to use
list(dic_labels) directly instead of the redundant comprehension, preserving the
same label ordering and values.
Source: Linters/SAST tools
|
|
||
| # Dump name is "dump_xxxx.sham" where xxxx is the timestep | ||
| model.do_vtk_dump(dump_folder + "/dump_{:04}.vtk".format(i), True) | ||
| model.do_vtk_dump(dump_folder + f"/dump_{i:04}.vtk", True) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find relevant files:"
fd -a 'run_sph_taylor_green_vortex.py|sph|do_vtk_dump' . | sed 's#^\./##' | head -200
echo
echo "Inspect target line:"
sed -n '160,200p' examples/sph/run_sph_taylor_green_vortex.py
echo
echo "Search do_vtk_dump definitions/usages:"
rg -n "def do_vtk_dump|do_vtk_dump\(" -S .Repository: Shamrock-code/Shamrock
Length of output: 7756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect pySPHModel binding around do_vtk_dump:"
sed -n '1,260p' src/shammodels/sph/src/pySPHModel.cpp
echo
echo "Inspect pyGSPHModel binding around do_vtk_dump:"
sed -n '1,220p' src/shammodels/gsph/src/pyGSPHModel.cpp
echo
echo "Inspect config files mentioning FBT003:"
filenames=$(git ls-files | rg '(^|/)(pyproject\.toml|ruff\.toml|\.ruff\.toml|setup.cfg)$|(^|/)(\.github/workflows/.*\.ya?ml)$' || true)
for f in $filenames; do
echo "--- $f"
rg -n "FBT003|ruff|flake8" "$f" || true
doneRepository: Shamrock-code/Shamrock
Length of output: 19717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search do_vtk_dump binding text:"
rg -n "do_vtk_dump|py::arg\\(\"add_patch_world_id\"|add_patch_world_id\\)" src/shammodels src -S
echo
echo "Inspect pySPHModel.cpp line ranges around model methods:"
rg -n "\.def\\(|do_vtk_dump|py::class_<T>" src/sham/models/sph/src/pySPHModel.cpp src/shammodels/sph/src/pySPHModel.cpp src/shammodels/sph/src -S
sed -n '220,340p' src/shammodels/sph/src/pySPHModel.cpp
sed -n '220,320p' src/shammodels/gsph/src/pyGSPHModel.cpp
echo
echo "Inspect config files for FBT rules with file patterns:"
for f in pyproject.toml ruff.toml .ruff.toml setup.cfg; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '/\\[tool\\.ruff\\|tool\\.ruff\\|FBT003\\|flake8-bandit\\|lint\\|select\\|per-file-ignores/p' | head -200
fi
doneRepository: Shamrock-code/Shamrock
Length of output: 33639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Config files:"
for f in pyproject.toml ruff.toml .ruff.toml setup.cfg; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f"
fi
done
echo
echo "pySPHModel do_vtk_dump binding context:"
sed -n '1370,1382p' src/shammodels/sph/src/pySPHModel.cpp
echo
echo "pyGSPHModel do_vtk_dump binding context:"
sed -n '398,406p' src/shammodels/gsph/src/pyGSPHModel.cpp
sed -n '130,220p' src/shammodels/gsph/src/pyGSPHModel.cppRepository: Shamrock-code/Shamrock
Length of output: 4099
🌐 Web query:
pybind11 keyword_only class method def no keyword_only arg name positional arguments FBT003
💡 Result:
The query involves two distinct technical concepts: pybind11's mechanism for keyword-only arguments and the FBT003 linting rule from flake8-boolean-trap (used by tools like ruff). These are not directly related, but they often intersect when developers work with C++ extensions that enforce positional arguments. 1. pybind11 Keyword-Only Arguments In pybind11, you can enforce keyword-only arguments using the py::kw_only() annotation when registering a function or method [1][2]. This annotation dictates that all subsequent arguments must be provided as keyword arguments [3]. For example: m.def("f", [](int a, int b) { /*... */ }, py::arg("a"), py::kw_only, py::arg("b")); This mimics Python's def f(a, *, b): syntax, where a can be positional or keyword-based, but b must be passed as a keyword [1]. Note that there have been historical issues regarding the combination of py::kw_only() with py::args, where incorrect usage can lead to static assertion failures during compilation [4][5]. 2. The FBT003 Linting Rule FBT003 is a specific rule from flake8-boolean-trap (integrated into ruff) that warns about "Boolean positional value in function call" [6]. It flags instances where a boolean value is passed positionally (e.g., func(True)) rather than as a keyword argument (e.g., func(flag=True)) to avoid ambiguity [7][6]. The intersection arises because many C/C++ extension functions—including those built with pybind11—do not accept keyword arguments for all parameters [7][8]. When a developer attempts to satisfy FBT003 by changing func(True) to func(flag=True), they may trigger a runtime TypeError because the underlying C++ code does not support keyword arguments for that specific parameter [7][9][8]. Common solutions for this conflict include: - Disabling the FBT rules entirely if the codebase frequently uses C-extensions [7][9]. - Using inline ignore comments (e.g., # noqa: FBT003) for specific calls [7][10]. - Configuring the linter's allow-list (e.g., flake8-boolean-trap.extend-allowed-calls in ruff) to ignore specific functions known to be positional-only [9][10]. Note that this configuration currently often requires explicit function names and may not resolve to class-level methods automatically [9].
Citations:
- 1: https://pybind11.readthedocs.io/en/stable/advanced/functions.html
- 2: https://pybind11.readthedocs.io/en/latest/advanced/functions.html
- 3: https://pybind11.readthedocs.io/en/stable/reference.html
- 4: [BUG]: py::args can not be combined with kw_only(), triggers static assert pybind/pybind11#5135
- 5: Allow kw-only args after a py::args pybind/pybind11#3402
- 6: https://flake8-boolean-trap.readthedocs.io/en/latest/readme.html
- 7: FBT003: ruff reports about function calls from C-extentions astral-sh/ruff#9287
- 8: FBT003: False positive with positional-only parameters astral-sh/ruff#3247
- 9:
flake8-boolean-trap.extend-allowed-callsdon't resolve class types astral-sh/ruff#11264 - 10: FBT003: Ignore by default django.db.models.Value astral-sh/ruff#10356
Use a targeted noqa for FBT003 at this binding call.
do_vtk_dump is bound as &T::do_vtk_dump without py::arg() naming, so the second bool parameter is not kw-only from pybind11’s signature. Passing True by keyword here would risk depending on pybind11’s default arg-name discovery and can fail for bound C++ methods.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 188-188: Boolean positional value in function call
(FBT003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/sph/run_sph_taylor_green_vortex.py` at line 188, Add a targeted
`FBT003` noqa directive to the `model.do_vtk_dump` call while keeping the
boolean argument positional. Do not convert `True` to a keyword argument or
broaden the suppression beyond this binding call.
Source: Linters/SAST tools
| # Read header and data | ||
| f.seek(0) | ||
| lines = [l for l in f.readlines() if not l.startswith("#")] | ||
| lines = [l for l in f if not l.startswith("#")] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check for exact usages of ambiguous variable in listed files:"
for f in examples/TO_MIGRATE/visualization/animate_sedov_csv.py examples/TO_MIGRATE/visualization/animate_sod_csv.py; do
if [ -f "$f" ]; then
echo "FILE $f"
sed -n '150,175p;75,95p' "$f"
else
echo "MISSING $f"
fi
done
echo
echo "Search nearby 'l' identifiers in these files:"
rg -n '\bl\b' examples/TO_MIGRATE/visualization/animate_sedov_csv.py examples/TO_MIGRATE/visualization/animate_sodov_csv.py 2>/dev/null || true
echo
echo "Check whether ruff E741 applies to list comprehension target 'l':"
python3 - <<'PY'
import ast, textwrap
mods = {
"sedov": 'lines = [l for l in f if not l.startswith("#")]',
"sod": 'lines = [l for l in f if not l.startswith("#")]',
}
for name, src in mods.items():
tree = ast.parse(textwrap.dedent(src))
for comp in ast.walk(tree):
if isinstance(comp, ast.ListComp):
print(name, "ListComp target:", type(comp.generators[0].target).__name__, "identifier:", comp.generators[0].target.id)
PYRepository: Shamrock-code/Shamrock
Length of output: 3461
Rename the ambiguous comprehension variable in both snapshot loaders.
l is an E741 variable name; use line in these lines so Ruff does not flag the file.
examples/TO_MIGRATE/visualization/animate_sedov_csv.py#L167-L167:lines = [line for line in f if not line.startswith("#")]examples/TO_MIGRATE/visualization/animate_sod_csv.py#L88-L88:lines = [line for line in f if not line.startswith("#")]
🧰 Tools
🪛 Ruff (0.16.0)
[error] 167-167: Ambiguous variable name: l
(E741)
📍 Affects 2 files
examples/TO_MIGRATE/visualization/animate_sedov_csv.py#L167-L167(this comment)examples/TO_MIGRATE/visualization/animate_sod_csv.py#L88-L88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/TO_MIGRATE/visualization/animate_sedov_csv.py` at line 167, Rename
the comprehension variable from l to line in both snapshot loaders:
examples/TO_MIGRATE/visualization/animate_sedov_csv.py lines 167-167 and
examples/TO_MIGRATE/visualization/animate_sod_csv.py lines 88-88, preserving the
existing filtering behavior.
Source: Linters/SAST tools
| or l.startswith(r"/*") | ||
| or l.startswith("\n") | ||
| ): | ||
| if not l.startswith((r"//", r"/*", r"/*", "\n")): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the nested else and if with elif.
Ruff 0.16 reports PLR5501 for Lines [37]-[38]. Use elif to keep the branch flat without changing its behavior.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 37-38: Use elif instead of else then if, to reduce indentation
Convert to elif
(PLR5501)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/check_pragma_once.py` at line 38, In the pragma-detection logic around
the visible startswith condition, replace the nested else/if branch with an elif
branch, preserving the existing conditions and behavior while eliminating the
PLR5501 warning.
Source: Linters/SAST tools
| fvers.close() | ||
| except: | ||
| None | ||
| pass |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle the file-read exception explicitly.
Ruff 0.16 reports S110 for this try/except/pass block. If a missing version file is expected, catch FileNotFoundError only. Log or re-raise other read errors so permission and I/O failures are not treated as a missing file.
🧰 Tools
🪛 Ruff (0.16.0)
[error] 10-11: try-except-pass detected, consider logging the exception
(S110)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/make_version_file.py` at line 11, Replace the broad
exception-swallowing try/except around the version-file read in the version-file
generation flow with an explicit FileNotFoundError handler. Preserve the
missing-file fallback, while logging or re-raising other read failures instead
of silently ignoring them.
Source: Linters/SAST tools
Comment on lines
+173
to
+174
| for i, l in enumerate(splt): | ||
| if l_start > 0 and not ("@author" in l): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Ruff violations in autocorrect.
Ruff reports E741 for the ambiguous variable name l and E713 for not ("@author" in l). Rename the variable and use the direct membership form.
Proposed fix
- for i, l in enumerate(splt):
- if l_start > 0 and not ("`@author`" in l):
+ for i, line in enumerate(splt):
+ if l_start > 0 and "`@author`" not in line:
break
- if "`@file`" in l:
+ if "`@file`" in line:
l_start = i
- if "`@author`" in l:
+ if "`@author`" in line:
l_end = i🧰 Tools
🪛 Ruff (0.16.0)
[error] 173-173: Ambiguous variable name: l
(E741)
[error] 174-174: Test for membership should be not in
Convert to not in
(E713)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/update_authors.py` around lines 173 - 174, In the autocorrect loop over
splt, rename the ambiguous l variable to a descriptive name and update its
references, then replace the negated membership expression with the direct
not-in form while preserving the existing condition behavior.
Source: Linters/SAST tools
Contributor
Workflow reportworkflow report corresponding to commit 3f7b25d Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportNo relevant changes found. You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review. Doxygen diff with
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.