`XFoil` wrapper's default settings are broken by three compounding bugs (and the test suite silently skips the one test that would catch it)

[benoitpaillard]

Summary

asb.XFoil(airfoil=..., Re=...).alpha(...) — the wrapper's actual documented default configuration — currently fails for most users, for three separate, compounding reasons. Individually each is a small bug; together they mean the happy path barely works at all outside of narrow test fixtures. I also found why this escaped detection: the one test that exercises a real XFoil binary skips silently on any exception, and the exception these bugs raise is exactly the kind it treats as "no functional XFoil available."

I have a branch with fixes for all three plus two new regression tests (17/17 passing, up from 14 passed/1 skipped on current master), verified in both normal and fully headless environments. Happy to open a PR — wanted to describe the findings first since it's a bigger diff than a one-line fix.

Environment: aerosandbox @ 60998b5c3935663a6ec11873511224e470c6aa6d (current master), Python 3.10.12, XFoil 6.99 (Ubuntu/Debian package xfoil 6.99.dfsg+1-3).

Bug 1: Crashes with "Cannot open display" on any fully headless machine

_default_keystrokes() unconditionally disables XFoil's graphics via plop / g / w 0.05. On a machine with zero display available at all ($DISPLAY unset — e.g. a CI runner, a Docker container, Streamlit Community Cloud), this build of XFoil still needs some display to talk to during PANE and hard-aborts:

 Cannot open display...aborting

(exit code 1, before ever reaching OPER/VISC/any actual analysis command).

Counter-intuitively, the "obvious" alternative fix — not disabling graphics — trades this for a different, also-fatal crash: this same XFoil build reproducibly SIGFPEs during PANE specifically when graphics are disabled first. I confirmed this directly with a controlled test, reproducing _default_keystrokes()'s exact keystrokes:

# Same case, only the graphics-disable block differs:
run_variant("plop/g graphics-disable (current default)", [...])   # -> returncode -8 (SIGFPE)
run_variant("no graphics-disable")                                 # -> returncode 0

So neither "graphics on" nor "graphics off" is safe on every environment with this XFoil build — the fix needs to actually ensure a display exists rather than picking one of the two.

Bug 2: cinc scheduled after pacc corrupts every polar with Re != 0

_default_keystrokes() sends pacc (start polar accumulation) before cinc (a toggle to include the Cpmin column). XFoil writes the polar file's column header at the moment pacc is issued, based on whichever column-toggles are active at that point — toggling cinc afterward changes the data rows XFoil subsequently writes, but not the already-written header. Result: every polar with Re != 0 (i.e. any viscous run, the normal case) comes back with one more data column than the header claims, and _run_xfoil raises:

XFoilError: XFoil output file is malformed; the header and data have different numbers of columns.

There's already a "monkey-patch" for this in _run_xfoil (search len(data) == 10 and len(columns) == 8), hardcoded to one specific combination (cinc + hinc/hinge-moment both active). It doesn't cover cinc alone (hinge_point_x=None), which hits the same underlying bug with a different column count (9 vs 10) that the hardcoded check doesn't match.

Minimal, isolated reproduction (bypassing the Python wrapper entirely, driving xfoil directly with the exact keystrokes in each order):

# ... same LOAD/PPAR/OPER/VISC setup in both cases, only cinc's position differs ...

# cinc AFTER pacc (current order) -> header/data mismatch:
#   header: 'alpha CL CD CDp CM Top_Xtr Bot_Xtr Top_Itr Bot_Itr'          (9 cols)
#   data:   '4.000 0.7168 0.00702 0.00117 -0.0566 -1.3898 0.3865 ...'    (10 values)

# cinc BEFORE pacc (fix) -> matches correctly:
#   header: 'alpha CL CD CDp CM Cpmin Top_Xtr Bot_Xtr Top_Itr Bot_Itr'    (10 cols)
#   data:   '4.000 0.7168 0.00702 0.00117 -0.0566 -1.3898 0.3865 ...'    (10 values)

Fix: move the cinc block to before the pacc block in _default_keystrokes(). This fixes the general case at the root rather than special-casing one symptom of it.

Bug 3: Top_Itr/Bot_Itr missing from the pre-initialized output dict

In _run_xfoil, output = {column: [] for column in [...]} is initialized with a fixed list of column names that does not include Top_Itr/Bot_Itr — even though these (boundary-layer iteration counts) are standard columns in any real viscous polar, not tied to any optional toggle. The monkey-patch mentioned in Bug 2 also uses this same incomplete list. Once Bug 2 is fixed (so the header is correctly parsed instead of hitting the malformed-columns error first), this becomes a KeyError: 'Top_Itr' instead.

Fix: add "Top_Itr" and "Bot_Itr" to that column list.

Why the existing test suite didn't catch bugs 2 and 3

test_xfoil.py's functional_xfoil fixture is the only test that runs a real XFoil binary; its own setup call uses the constructor's actual defaults (hinge_point_x=0.75, i.e. hinge-moment + cinc both active — exactly Bug 2's trigger condition) wrapped in a broad except Exception: pytest.skip(...), intended to skip gracefully when no XFoil binary is available. Because Bug 2 raises an XFoilError (not FileNotFoundError or similar), it's indistinguishable to that fixture from "XFoil isn't installed" — so the one test that would exercise real default-settings behavior has been silently skipping instead of failing:

$ pytest aerosandbox/aerodynamics/aero_2D/test_aero_2D/test_xfoil.py -v
...
test_xfoil_alpha_with_hinge_point_none SKIPPED
14 passed, 1 skipped

The other fixture-based tests (test_polar_parsing_without_binary, test_polar_parsing_with_hinge_moment_disabled, etc.) use hand-authored polar text as their fixtures, which happen to already be internally consistent (and happen to omit Top_Itr/Bot_Itr too) — so they never exercised either bug.

Verification

With all three fixed, plus two new regression tests (one exercising real-default-settings against a real XFoil binary — test_xfoil_alpha_with_default_settings — and one exercising the missing-columns case via a fixture that includes Top_Itr/Bot_Itr):

$ pytest aerosandbox/aerodynamics/aero_2D/test_aero_2D/test_xfoil.py -v
...
17 passed in 2.25s

...and identically with $DISPLAY unset (env -u DISPLAY pytest ...), confirming Bug 1's fix independently:

17 passed in 3.42s

Also ran the broader airfoil/polar-generation test suite (test_airfoil_inviscid.py, test_generate_polars_data_handling.py, test_airfoil_polar_generation.py) — no regressions.

Proposed fix

I have a local branch with all three fixes + the two new tests (diff: ~94 lines across xfoil.py and test_xfoil.py). Let me know if a single PR covering all three is preferred, or if you'd rather they be split up.

[benoitpaillard]

Hi Peter, this was automated and the tone might sound a bit weird, sorry for that :|