Skip to content

MNT: store ref_factor on rocket aero surface components (#561) - #1129

Open
thatrandomasiandev wants to merge 1 commit into
RocketPy-Team:developfrom
thatrandomasiandev:mnt/561-ref-factor-components
Open

MNT: store ref_factor on rocket aero surface components (#561)#1129
thatrandomasiandev wants to merge 1 commit into
RocketPy-Team:developfrom
thatrandomasiandev:mnt/561-ref-factor-components

Conversation

@thatrandomasiandev

Copy link
Copy Markdown

Summary

  • Extend Components.component_tuple with ref_factor (default 1.0) and pass it through Components.add, serialization, and call sites that unpacked (component, position).
  • Compute and store ref_factor = (surface.rocket_radius / rocket.radius)**2 when adding aero surfaces; use the stored value in evaluate_center_of_pressure and rocket prints.
  • Add unit tests for Components storage/round-trip and the Rocket add path.

Fixes #561

Test plan

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/unit/rocket/test_components.py tests/unit/rocket/test_rocket.py -q
  • PYTEST_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

@thatrandomasiandev
thatrandomasiandev requested a review from a team as a code owner August 11, 2026 01:38
@Gui-FernandesBR
Gui-FernandesBR force-pushed the mnt/561-ref-factor-components branch from ab19b9a to f881d7e Compare August 14, 2026 00:16
@Gui-FernandesBR
Gui-FernandesBR force-pushed the mnt/561-ref-factor-components branch from f881d7e to ebe5b8b Compare August 15, 2026 12:16
@Gui-FernandesBR

Copy link
Copy Markdown
Member

This one needs a different approach rather than a rebase. Two compatibility problems, and the first is what is failing CI:

1. It breaks .rpy files that already exist.

FAILED tests/unit/test_utilities.py::test_load_from_rpy
ValueError: not enough values to unpack (expected 3, got 2)   (rocket.py:2345)

Rocket.from_dict destructures three elements out of data["aerodynamic_surfaces"], but every file serialized before this PR stores two. The namedtuple's defaults=(1.0,) covers Components.add, it does not cover destructuring of already-stored data.

2. It breaks the public API. for surface, position in rocket.aerodynamic_surfaces: is one of the most common idioms in user notebooks and it now raises ValueError. The PR description marks this as not a breaking change; it is one.

A backward-compatible shape would be to keep the 2-tuple iteration contract intact and expose ref_factor some other way (an attribute on the component, or a separate lookup), and to make from_dict accept both the 2- and 3-element forms. I would like to see test_load_from_rpy and the 2-tuple idiom both still working before this goes in.

3. Lint is one ruff format away — rocketpy/rocket/components.py, the self._components.append(...) call now fits on one line:

self._components.append(self.component_tuple(component, position, ref_factor))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MNT: save ref_factor value when adding aero surfaces to a Rocket

2 participants