MNT Set up vulture and remove dead code it flagged#65
Conversation
Add vulture configuration in pyproject.toml plus a whitelist file for pytest fixtures and other dynamically-referenced names. Running `vulture` with no args now exits cleanly. Also clean up the unambiguous dead code that the initial scan flagged: - Remove unused cmd_default in cli.py (dispatch is by argparse subcommand) - Drop write-only best_conn local in core.py - Rename unused loop var r_iter -> _ in flatten/algorithm.py - Drop unused `call` import in tests/test_core.py - Rename ignored lambda params subj -> _subj in tests/test_freesurfer.py
These functions are not referenced anywhere in the package (nor exported from flatten/__init__.py) and have no callers: - SurfaceFlattener.compute_distance_error / count_flipped: wrapper methods duplicating the private _compute_distance_error_jit and count_flipped_triangles helpers that the algorithm actually uses. - setup_heat_geodesic / compute_heat_distance: heat-method alternative never wired into the pipeline; production uses graph Dijkstra. - compute_graph_distance: single-source variant superseded by the batched compute_kring_geodesic_distances path. - compute_both_energies_edges: edge-list variant superseded by the k-ring vectorized energy functions. Also trims distance.py's module docstring and section header now that the heat-method block is gone.
There was a problem hiding this comment.
Code Review
This pull request focuses on code cleanup and the integration of the Vulture static analysis tool. It removes various unused functions, variables, and imports across the codebase, including the removal of the heat method for distance computation and several unused utility methods. Additionally, it configures Vulture in pyproject.toml and introduces a whitelist for pytest fixtures. Feedback suggests that the scripts directory included in the Vulture configuration may not exist, potentially causing the tool to fail.
| check-hidden = true | ||
|
|
||
| [tool.vulture] | ||
| paths = ["autoflatten", "scripts", ".vulture_whitelist.py"] |
There was a problem hiding this comment.
scripts/ does exist — it contains 4 Python files (demo_flatten_animation.py, make_fsaverage_cut_template.py, make_subject_cut_template.py, plot_all_flatmaps.py). Including it in the vulture paths is intentional: it lets vulture see that helper functions like setup_freesurfer, identify_surface_components, and save_json are referenced (only from these scripts), so they don't get flagged as dead. vulture runs to completion with exit code 0 against the current config.
Generated by Claude Code
Summary
pyproject.toml(paths, min_confidence 80,ignore_names = ["kwargs", "trim"]to suppress the backend**kwargsinterface pattern and the documented backward-compattrimno-op inviz.plot_patch) plus.vulture_whitelist.pyfor pytest fixtures that are looked up by name rather than imported.vulture>=2.14in thedevdependency group. Runningvulturefrom the repo root now exits 0.cmd_defaultincli.py(dispatch happens via argparse subcommand, nothing called it).best_connlocal incore.py.r_iter→_inflatten/algorithm.py.callimport intests/test_core.py.subj→_subjintests/test_freesurfer.py(monkeypatch.setattrcallbacks that ignore their argument).flatten/subpackage (not in__all__, no callers anywhere):SurfaceFlattener.compute_distance_error/count_flipped— wrappers duplicating_compute_distance_error_jitandcount_flipped_trianglesthat the algorithm uses directly.setup_heat_geodesic/compute_heat_distance— heat-method path never wired into the pipeline; the production code uses graph Dijkstra.compute_graph_distance— single-source variant superseded bycompute_kring_geodesic_distances.compute_both_energies_edges— edge-list variant superseded by the k-ring vectorized energy functions.distance.py's module docstring and section header now that the heat-method block is gone.Test plan
vulture(run from repo root) exits 0vulture autoflatten scripts --exclude "autoflatten/_version.py,autoflatten/tests/"exits 0 (no API-level dead code remaining)pytestpasses (the removed functions had no tests; the renamed lambda params and droppedcallimport keep test semantics identical)autoflatten --helpstill lists all subcommands (run,project,flatten,plot-flatmap,plot-projection,render-snapshots)Generated by Claude Code