Polars follow table_format through the node-table I/O, and lose csv from their names

[1-Bort-1]

TL;DR

With table_format=:arrow, obj_to_yaml, surfplan_to_aero_yaml and generate_airfoils now write polars/{j}.arrow next to the .arrow Cp/cf tables, and load_polar_data reads it. Until now every polar was CSV whatever the setting, so BeyondTheSim's SK100 run with table_format: arrow still got CSV polars. There is no second Arrow path: the polar writers and the loader go through read_node_table / write_node_rows, which now carry column names and an optional delta column. Since a polar is no longer necessarily CSV, the writers are renamed write_polar / write_polar_matrix and the YAML key polar_file_path (breaking, for the coming release).

What changed

• One table reader and writer. read_node_table(path) returns (alpha, delta, values, columns). delta is nothing when the table has no delta column, and the columns are found by header name (case-insensitive, any order). A POLAR_MATRICES table (alpha, delta, Cl, Cd, Cm) and a POLAR_VECTORS table (alpha, Cd, Cs, Cl, Cm) are therefore both ordinary node tables. write_node_rows(...; columns) writes either. csv_fields (from Write polar CSV coefficients at 16 significant digits instead of 4 decimals #369) moved beside it and is now the only CSV row formatter.
• Polar writers and loader. write_polar (both methods) and write_polar_matrix are now a few lines each on top of write_node_rows, CSV or Arrow as the path's suffix says. load_polar_data lost its 90-line hand parser and calls read_node_table. It still warns and returns (nothing, INVISCID) for a missing, empty or malformed file; the eight existing test_load_polar_data.jl cases pass unchanged.
• Datasets. generate_airfoils names the polar polars/{j}.{table_format}. migrate_node_tables also converts the polar, so switching format on a cached dataset rewrites it instead of re-running NeuralFoil. surfplan_to_aero_yaml's reuse branch now calls it too; before, it returned the old YAML untouched. resolve_airfoil and resolve_aero_geometry take table_format as well, so the awesIO neuralfoil route writes {id}.arrow; it wrote {id}.csv whatever was asked. Existing CSV datasets load as before.
• Renames (BREAKING). write_polar_csv → write_polar, write_polar_matrix_csv → write_polar_matrix, and the geometry YAML key csv_file_path → polar_file_path, as Rename write_polar_csv, write_polar_matrix_csv and csv_file_path now that polars can be Arrow #373 proposed and @1-Bart-1 asked for here. The old function names are gone. The old key is still read by Wing, because BeyondTheSim's datasets and awesIO-style inputs carry it, and migrate_node_tables writes it back as polar_file_path when it converts a dataset. Every YAML in data/ and test/ now uses the new key; a test loads one with the old key.
• POLAR_VECTORS is covered. Its only difference from a node table is the missing delta column, which the optional delta handles.

Where it differs from the issue's sketch

• Arrow layout: one list column plus metadata, not one column per value. The issue suggested named value columns. I tried one Arrow column per value first. Arrow then compiles afresh for every column count: the first write of a 151-column node table took 1.64 s against 0.001 s for the second, and node counts differ per airfoil. So the layout stays as it was, alpha, delta and a list column values, and the column names go into the schema metadata entry columns. That gives one compile for every width. Arrow files written before this PR have no metadata and read with n0, n1, … names (tested). The cost is that pyarrow shows a polar as a values list plus metadata, not as Cl/Cd/Cm columns.
• Angles are rounded to 16 significant digits in both formats. Node-table CSV used to print values at full precision with join(row, ","); it now uses csv_fields, as the issue asked. That broke the existing arrow.delta_range == back.delta_range check: CSV now writes rad2deg(deg2rad(3)) as 3, while Arrow stored 2.9999999999999996. write_node_rows now rounds the degree angles to the same 16 digits for Arrow as well, and the check passes again unchanged. The coefficients themselves differ between the formats only past the 16th digit.

Found on the way, not changed

• read_csv_columns parses every column as a number, and throws on a row whose field count differs from the header. The old load_polar_data parsed only alpha/cl/cd/cm, so a CSV polar with an extra non-numeric column (a source label, say) now warns and loads as INVISCID. Nothing in this repo or its datasets writes such a column. The CHANGELOG says so, and a test pins it.

Searched first for: write_polar, load_polar_data, resolve_airfoil, read_node_table, write_node_rows, csv_fields, table_format, csv_file_path.

Verification

• Reproduced first: n/a, new feature. Before this PR, polars were CSV regardless of table_format (endswith("polars/1.csv", ".arrow") fails in the migration test).
• Red on Write polar CSV coefficients at 16 significant digits instead of 4 decimals #369's src with the new tests: test_airfoil_aero.jl BoundsError (only three return values from read_node_table), test_surfplan.jl polars/1.arrow missing, test_obj_adapter.jl polar still polars/1.csv. Red for the renames on the previous head's src: test_obj_adapter.jl 49 passed, 2 failed, 7 errored (resolve_aero_geometry has no table_format, the key is not rewritten), test_yaml_geometry.jl 165 passed, 15 failed, 2 errored (polar_file_path not read).
• Green (juliaserver, fresh session, after merging main): test_airfoil_aero.jl 134/134, test_yaml_geometry.jl 182/182 (includes test_load_polar_data.jl and test_wing_constructor.jl), test_obj_adapter.jl 63/63, test_surfplan.jl 8/8, test_wing_geometry.jl 302/302.
• Docs build clean in a fresh session (only the existing size warnings) · no REUSE in this repo · merged with main (the CHANGELOG conflict kept both sides).
• Local CI mirror (Pkg.test(), Julia 1.13, fail-fast) on ce475fc: PASS in 5 min, exit 0 · GitHub CI on 0223e32 (merge of main, CHANGELOG only): PASS on every cell, Windows included. The Windows 1.12 cell failed once on ce475fc with the flaky ForwardDiff POLAR_MATRICES check (0.0571 < 1e-4, test_forwarddiff.jl:99), reopened as Flaky POLAR_MATRICES forwarddiff test on Windows / Julia 1.12 (knot-proximity in piecewise-linear polar interpolation) #360; this PR touches neither the solver nor that test · jetls: not run, not installed on the box.
• Benchmark: n/a. The only timing is the per-width compile above. Reading a 600 × 151 Arrow node table allocates 1.80 MB, down from 2.52 MB, now that read_node_table returns a view of the value columns (@allocated, second call).
• Risk: a downstream script calling write_polar_csv or reading info["csv_file_path"] from a freshly written YAML; both are named in the CHANGELOG as breaking.

Scope

+447 / −359 across 26 files against main. About 80 of those lines are the key rename in the data/ and test/ YAMLs, one line each. Net growth sits in section_aero.jl (+45: the named-column reader split into read_arrow_columns / read_csv_columns) and the tests. yaml_geometry.jl shrinks by 100 lines now that load_polar_data reuses the reader, and the three polar writers shrink to a few lines each. Builds on #369, now merged. Closes #373.

Closes #371 · task VortexStepMethod.jl-371

[1-Bort-1]

Independent review (advisory)

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

Good

• Delivers what the card promises: generate_airfoils names the polar polars/{j}.{table_format}, and migrate_node_tables now includes csv_file_path. Checked in geometry_gen.jl:41 and obj_to_yaml.jl:129.
• Keeps one codepath: the three polar writers, convert_node_table and load_polar_data all go through read_node_table/write_node_rows, and csv_fields is now the only CSV row formatter. Grep finds no other polar parser in src.
• Removes the 90-line hand parser in load_polar_data. It also fixes the old load_polar_data docstring, which sat on assemble_polar_matrix; that function now has its own accurate docstring.
• write_polar_csv(sols) keeps alpha in radians and write_node_rows does the degree conversion. The NeuralFoil method passes deg2rad. first. Both land in degrees, the same as before.
• The legacy Arrow read falls back to n0, n1, … names when there is no metadata, and a test covers it (test_airfoil_aero.jl:363). The existing read_section_aero call that unpacks three of the four return values still works.
• The surfplan reuse branch was a real gap: it returned the old YAML untouched. It now migrates, and a test covers it (test_surfplan.jl:41-46).
• The card explains why it departs from the issue sketch (per-width Arrow compile) and gives measured timings, so the metadata-column layout is justified.

Not good

• src/section_aero.jl:169 — data[:, value_cols] copies the whole value matrix after read_arrow_columns has already filled data. Every Cp/cf load in read_section_aero now allocates the node table twice, on the path that Arrow is meant to speed up.
• src/section_aero.jl:213 — read_csv_columns parses every column as Float64. A polar CSV with an extra non-numeric or empty column now fails, where the old load_polar_data parsed only alpha/cl/cd/cm, and load_polar_data then falls back to INVISCID with only a warning. The card mentions the stricter field count but not this, and the CHANGELOG says 'CSV polars load as before'.
• src/airfoil_aero/airfoil_solvers/common.jl:44 — Destructuring a generator of getfield comprehensions over a tuple of field names is the 'clever comprehension chain' that §3 rules out. Four plain [sol.alpha for sol in converged] lines, or one loop, would read at a glance.
• docs/src/airfoil_pipeline.md:100 still lists polars/{j}.csv, while the paragraph at :109 says the polar follows table_format. The bullet should read polars/{j}.{table_format}, the same as obj_to_yaml's docstring.
• CHANGELOG line 29, "and now also surfplan_to_aero_yaml", only makes sense next to the previous version (§7 countermand). State what both functions do.
• The test at test_airfoil_aero.jl:346 and :356 states the expected column names twice with two different ternaries. Put the expected columns in the writers table next to each writer.
• write_node_rows builds hcat(angles..., values), a full copy of the node matrix, only to format CSV rows. This runs on every node-table write, not just for polars.
• The card lists _csv names that no longer fit and a csv_file_path key that can now point at an .arrow file. No follow-up issue is linked for that breaking rename.

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

[1-Bort-1]

MINOR: data[:, value_cols] copies the whole value matrix after read_arrow_columns has already filled data. Every Cp/cf load in read_section_aero now allocates the node table twice, on the path that Arrow is meant to speed up.

[1-Bort-1]

Fixed in e0a1f58: read_node_table returns a view of the value columns. A 600 × 151 Arrow table now allocates 1.80 MB to read, down from 2.52 MB. write_node_rows also no longer hcats before writing the CSV rows.

[1-Bort-1]

MINOR: read_csv_columns parses every column as Float64. A polar CSV with an extra non-numeric or empty column now fails, where the old load_polar_data parsed only alpha/cl/cd/cm, and load_polar_data then falls back to INVISCID with only a warning. The card mentions the stricter field count but not this, and the CHANGELOG says 'CSV polars load as before'.

[1-Bort-1]

Kept strict: e0a1f58 corrects the CHANGELOG line and the card, and a new test_load_polar_data.jl case pins that a non-numeric column loads as INVISCID. Nothing here writes such a column.

[1-Bort-1]

MINOR: Destructuring a generator of getfield comprehensions over a tuple of field names is the 'clever comprehension chain' that §3 rules out. Four plain [sol.alpha for sol in converged] lines, or one loop, would read at a glance.

[1-Bort-1]

Fixed in e0a1f58: four plain comprehensions.

[1-Bort-1]

Local full suite: PASS (4 min, Julia 1.13.0, one cell of the matrix)

[codecov]

Codecov Report

❌ Patch coverage is 98.92473% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/airfoil_aero/airfoil_io.jl	87.50%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[1-Bart-1]

fix the first two found on the way points in this PR. we are already preparing a breaking release, so perfect timing.

[1-Bart-1]

Should we reopen the flaky windows forwarddiff test issue and assign it to 1-Bort-1, bc it is failing on windows again.

[1-Bort-1]

Fixed in fb5e74c; closes #373.

[1-Bort-1]

Yes: reopened #360 and assigned it to 1-Bort-1. It failed on Windows / Julia 1.12 in #372's CI with 0.0571 < 1e-4 at test_forwarddiff.jl:99; #372 doesn't touch that code.