Skip to content

Give every untyped empty container its element type - #338

Merged
1-Bart-1 merged 1 commit into
mainfrom
agent/147-refactoring-part-iii
Sep 16, 2026
Merged

1-Bart-1 merged 1 commit into
mainfrom
agent/147-refactoring-part-iii

Conversation

@1-Bort-1

Copy link
Copy Markdown
Contributor

TL;DR

vertices = [], plots = [], filaments_plot = [] and the other bare []/Any[] literals now say what they hold: in src, the Makie extension, tests and the stall example. This is the first box of #147. Typing the list in the Makie extension surfaced a real inconsistency: plot!(ax, body) drawn as flat panels returned one list per panel, while every other branch returned a flat list. Now all branches return one flat Vector{Makie.AbstractPlot}.

What changed

  • read_faces builds Vector{Vector{Float64}} / Vector{Vector{Int64}}, and calculate_filaments_for_plotting builds the Vector{Tuple{Vector{Float64}, Vector{Float64}, String}} its docstring already promised. Both used to return Vector{Any}.
  • Makie extension: both plot! methods collect into Makie.AbstractPlot[]. The static body path now uses append! instead of push!, because pushing a panel's plot vector into a typed list throws. That is the one behaviour change and it has a changelog line. plot_polars types its polar_data_list and cm_data_list.
  • VSMSettings.wings defaults to WingSettings[] instead of a [] that kwdef converted anyway.
  • Tests: the Python-port helpers in test/thesis_oriol_cayon.jl get Dict{String, Any}[], Vector{Float64}[] and so on. Three push! loops became comprehensions, and plot_polars(Any[], Any[], …) became plot_polars(Solver[], BodyAerodynamics[], …).
  • examples/stall_model.jl adds each rib to the wing as it reads the CSV row, instead of filling an untyped rib_list first.

Searched for = [], Any[] and bare [] literals across src, ext, test, examples, docs/src, scripts and README. None are left.

Left as they are

  • Vector{Any}[] rows in geometry_gen.jl, obj_to_yaml.jl and SurfplanAdapter.jl. These are YAML table rows that mix an Int id with Float64 coordinates. Typing them as Float64 would print the id as 3.0.
  • Tuple[], NamedTuple[] and Ref{Any}: these are typed, only abstractly.
  • The extension's global observable stores are Dict(). PANEL_MESH_OBSERVABLES is keyed by (body_id, panel_idx) in two places and by a bare panel objectid in the third (plot(panel)). The mixed key is the thing to fix there, and it is a different concern from typed vectors.

The rest of #147

The two renames touch about 900 names, and va=/v_a= are public keywords, so they are queued as a stack on top of this PR:

  1. Rename the *_array locals to say whether they are a vector or a... #336: *_array locals to vector/matrix names (no public API)
  2. One name per apparent-wind quantity: va, v_a, _va and va_*... #337: one va convention. It starts by asking whether the public va= keyword, set_va! and Panel.va follow the convention agreed on Refactoring part III #147.

Not stacked on #325, which also edits obj_geometry.jl and test_kite_geometry.jl, but in hunks that don't overlap these.

Verification

  • Reproduced first: the new checks on main's code evaluate Any[[0.0, 0.0, 0.0], …] isa Vector{Vector{Float64}}, Any[(…, "magenta"), …] isa Vector{Tuple{…}}, Any[Any[Mesh{…}, …]] isa Vector{Makie.AbstractPlot} and 20 == 40 (nested per-panel list)
  • test/panel/test_panel.jl, test/ram_geometry/test_kite_geometry.jl: 3 failed before, green after. test/plotting/test_plotting.jl: 3 failed before, green after.
  • runtests.jl filtered to panel, kite geometry, settings, obj adapter, body aerodynamics and plotting: 5085 passed, 1 broken (the existing @test_broken in Kite Geometry). yaml_geometry (includes wing deformation): 178 passed. examples/stall_model.jl runs to the end.
  • These ran in a bare julia +1.12 --project=test, not juliaserver: the box's REPL sweeper stopped this task's session every minute (1-Bart-1/Agents#413)
  • Up to date with main (2b312ea) · no new or renamed public symbol, docs build not run · no REUSE in this repo
  • Local CI mirror: started, not finished at the time of writing · GitHub CI: not run yet, starts when this PR opens
  • Risk: a downstream caller that indexed plot!(ax, body) as plots[i] per panel now gets individual plots.

Scope

+60 / −52 across 11 files. The growth is the three new type checks and the changelog line; the rest is one-for-one replacements.

Refs #147 · task VortexStepMethod.jl-147

`vertices = []`, `plots = []`, `filaments_plot = []` and the like become
`Vector{Float64}[]`, `Makie.AbstractPlot[]`, `Tuple{...}[]`, in src, the Makie
extension, tests and the stall example. `plot!(ax, body)` drawn as flat panels
now appends each panel's plots, so every branch returns one flat
`Vector{Makie.AbstractPlot}`.

Refs #147

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:ci Agent task state and removed agent:queued Agent task state labels Sep 16, 2026

@1-Bort-1 1-Bort-1 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review (advisory)

Verdict: APPROVE WITH COMMENTS · 1 inline, 0 off the diff

Good

  • The commit's own diff (HEAD~1..HEAD) is 11 files, +60/−52, and matches the card's file list and scope exactly
  • A search of src, ext, test, examples and docs for = [], Any[], ([] and , [] finds nothing left, which backs the card's claim
  • The static-body change from push! to append! is correct: the Panel plot! returns its own list (mesh + border), so the old result was nested and the new one is flat like the observable and airfoil branches; there is a changelog line and a test (length == 2 * panels)
  • The declared element types match what gets pushed: generate_polar_data builds polar_data from vectors plus distributions, and the thesis helpers' ringvec/wingpanels dicts hold only Vector{Float64} values
  • calculate_filaments_for_plotting now returns the tuple type its docstring already promised, and a new unit test pins it rather than a slow integration test
  • The plot_polars(Solver[], BodyAerodynamics[], …) test calls still fit the untyped solver_list/body_aero_list signature, so no API changed
  • Deferring the va/*_array renames to #336 and #337 keeps this PR to one idea, as §1 asks

Not good

  • examples/stall_model.jl:35 — With rib_list gone, a stray blank line now separates df = CSV.read(...) from its only use, and the # Create wing geometry comment now also sits over the CSV read loop. Moving the read down next to the loop would keep reading the data and building the wing in one place.
  • The mixed PANEL_MESH_OBSERVABLES key ((body_id, panel_idx) vs. a bare panel objectid) is a real inconsistency found on the way, but it is only described under 'Left as they are' with no linked issue, which §5 asks for
  • NamedTuple{(:cmx, :cmy, :cmz), NTuple{3, Vector{Float64}}}[] at ext line 934 is a long inline type; it is correct, but a reader has to parse it to see it just means 'cm columns'
  • Line 282 of thesis_oriol_cayon.jl repeats the Dict{String, Any}[] literal from line 127, so the two resets have to be kept in step by hand
  • The only checks listed are local and filtered, and the local CI mirror had not finished; the full suite and GitHub CI are still unconfirmed

claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.

Comment thread examples/stall_model.jl
push!(rib_list, (LE, TE, LEI_AIRFOIL_BREUKELS,
lei_poly_coeffs(row.d_tube, row.camber)))
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR: With rib_list gone, a stray blank line now separates df = CSV.read(...) from its only use, and the # Create wing geometry comment now also sits over the CSV read loop. Moving the read down next to the loop would keep reading the data and building the wing in one place.

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@1-Bart-1
1-Bart-1 merged commit a95bfac into main Sep 16, 2026
7 checks passed
@1-Bart-1
1-Bart-1 deleted the agent/147-refactoring-part-iii branch September 16, 2026 22:01
@1-Bort-1 1-Bort-1 added agent:done Agent task state and removed agent:review Agent task state labels Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:done Agent task state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants