-
-
Notifications
You must be signed in to change notification settings - Fork 0
Change simulation encoding workaround to official rocketpy encoders #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis update replaces the use of a generic attribute extraction utility with a new, configurable encoding approach for RocketPy objects. It introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant ServiceClass as Service (e.g., RocketService)
participant RocketPyObj as RocketPy Object
participant Encoder as rocketpy_encoder
participant Config as DiscretizeConfig
participant Simulation as SimulationView
ServiceClass->>RocketPyObj: Access instance (e.g., self.rocket)
ServiceClass->>Config: Get config (e.g., DiscretizeConfig.for_rocket())
ServiceClass->>Encoder: rocketpy_encoder(RocketPyObj, Config)
Encoder->>RocketPyObj: Discretize and encode attributes
Encoder-->>ServiceClass: Encoded dict
ServiceClass->>Simulation: SimulationView(**encoded_dict)
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
🔇 Additional comments (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates simulation encoding to use the official RocketPy encoders and introduces a new DiscretizeConfig for improved dynamic attribute support across simulation models.
- Replaces get_instance_attributes with rocketpy_encoder and config objects in service files.
- Updates simulation view classes (Rocket, Motor, Flight, Environment) to use pydantic’s ConfigDict and dynamic extra attributes.
- Refactors the utils module by adding DiscretizeConfig and enhancing the rocketpy_encoder logic to handle callable functions.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/views/rocket.py | Updated RocketSimulation with new field types and removal of default for coordinate_system_orientation. |
src/views/motor.py | Updated MotorSimulation to extend ApiBaseView with new dynamic attributes and altered defaults. |
src/views/flight.py | Revised FlightSimulation to integrate new encoding logic and dynamic flight attribute support. |
src/views/environment.py | Modified EnvironmentSimulation to include updated function attribute fields. |
src/utils.py | Introduced DiscretizeConfig and enhanced rocketpy_encoder to work with official RocketPyEncoder. |
src/services/*.py | Updated service methods to replace get_instance_attributes with rocketpy_encoder and proper config usage. |
Comments suppressed due to low confidence (2)
src/views/rocket.py:29
- Removing the default value ('tail_to_nose') for coordinate_system_orientation in RocketSimulation may break downstream behavior that expects this default; please confirm if this change is intentional or reinstate the default.
coordinate_system_orientation: Optional[str] = None
src/views/motor.py:35
- The default value for coordinate_system_orientation has been removed (previously 'nozzle_to_combustion_chamber'); confirm that this modification is intended as it may affect components relying on that default.
coordinate_system_orientation: Optional[str] = None
@coderabbitai review |
✅ Actions performedReview triggered.
|
Pull Request Overview
This PR shifts the simulation encoding approach to utilize the official RocketPy encoders and a new DiscretizeConfig for improved dynamic attribute support across simulation models. Key changes include:
Refactor
Documentation