MNT: store ref_factor on rocket aero surface components (#561) - #1129
Conversation
ab19b9a to
f881d7e
Compare
f881d7e to
ebe5b8b
Compare
|
This one needs a different approach rather than a rebase. Two compatibility problems, and the first is what is failing CI: 1. It breaks
2. It breaks the public API. A backward-compatible shape would be to keep the 2-tuple iteration contract intact and expose 3. Lint is one self._components.append(self.component_tuple(component, position, ref_factor)) |
Rocket.from_dict unpacked three fields from every serialized entry, so a .rpy file written before ref_factor was stored failed to load with "not enough values to unpack (expected 3, got 2)" -- which is what the committed fixture tests/fixtures/utilities/flight_calisto_robust.rpy is, and what every file a user already has saved is too. Regenerating the fixture would have turned the suite green while leaving those files unreadable, so the three loops take a trailing star instead and read either length. Nothing is lost by ignoring the stored factor: add_surfaces derives it again from the surface's own radius. Two tests that reached develop after this branch opened, in RocketPy-Team#1169 and RocketPy-Team#1170, iterate Components expecting pairs, and now get triples. Both take the star as well, so a later field does not break them again. Also fixes what CI would have failed on regardless of the above: pylint C0415 for the two rocketpy imports inside test functions, now at the top of the module, and one ruff formatting difference in components.py. Simulation results are unchanged -- the 18 acceptance tests pass, and the factor is the same number as before, computed once when the surface is added rather than at every lift evaluation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1129 +/- ##
===========================================
+ Coverage 91.21% 91.24% +0.02%
===========================================
Files 131 131
Lines 17618 17619 +1
===========================================
+ Hits 16070 16076 +6
+ Misses 1548 1543 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ltistage #1129 landed on develop while this branch was open and made Components yield (component, position, ref_factor). This branch reads that collection in five places, so 29 of its tests failed with "too many values to unpack (expected 2)": axial_extent, two surface comprehensions and one draw loop in multistage, plus a dict(composed.aerodynamic_surfaces) in the tests, which failed differently again ("dictionary update sequence element #0 has length 3"). All five take a trailing star now, so a further field does not break them a third time. Addressing the review, in the same order the comments were left: - multistage.py was 810 lines holding four classes and a helper, and is now a package of one class per file, following the aero_surface layout that is already in rocketpy/rocket/. __init__ re-exports all five names, so every existing import path keeps working unchanged -- including the notebook's and the docstring cross-reference to axial_extent. - multistage_mission_test_report.ipynb is removed. It was the author's own working file, as they confirmed, and nothing referenced it. That leaves the two CI failures: ruff format over the notebook, multistage and its tests, and the build-docs warning that multistage_mission.ipynb sat in no toctree, which is now listed under Special Case Simulations next to the sensors notebook. Removing the report notebook takes care of the second warning of the two, since it was unreferenced as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Components.component_tuplewithref_factor(default1.0) and pass it throughComponents.add, serialization, and call sites that unpacked(component, position).ref_factor = (surface.rocket_radius / rocket.radius)**2when adding aero surfaces; use the stored value inevaluate_center_of_pressureand rocket prints.Fixes #561
Test plan
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/unit/rocket/test_components.py tests/unit/rocket/test_rocket.py -qPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/unit/rocket/ tests/unit/test_rail_buttons_bending_moments.py tests/integration/test_sensor.py tests/unit/simulation/test_flight.py -q