-
-
Notifications
You must be signed in to change notification settings - Fork 1
MNT: adapt discretization schema to RocketPy encoder. #52
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
Changes from all commits
9339a43
fb23890
ba7c28b
7c7f805
6ecc358
5eab472
8450082
c295651
42afc99
4368487
26f510d
52e09ca
a15232d
a8862c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,14 +11,14 @@ | |||||||||||||||
| Fins as RocketPyFins, | ||||||||||||||||
| Tail as RocketPyTail, | ||||||||||||||||
| ) | ||||||||||||||||
| from rocketpy.utilities import get_instance_attributes | ||||||||||||||||
|
|
||||||||||||||||
| from src import logger | ||||||||||||||||
| from src.models.rocket import RocketModel, Parachute | ||||||||||||||||
| from src.models.sub.aerosurfaces import NoseCone, Tail, Fins | ||||||||||||||||
| from src.services.motor import MotorService | ||||||||||||||||
| from src.views.rocket import RocketSimulation | ||||||||||||||||
|
|
||||||||||||||||
| from src.views.motor import MotorSimulation | ||||||||||||||||
| from src.utils import collect_attributes | ||||||||||||||||
|
|
||||||||||||||||
| class RocketService: | ||||||||||||||||
| _rocket: RocketPyRocket | ||||||||||||||||
|
|
@@ -107,8 +107,11 @@ def get_rocket_simulation(self) -> RocketSimulation: | |||||||||||||||
| Returns: | ||||||||||||||||
| RocketSimulation | ||||||||||||||||
| """ | ||||||||||||||||
| attributes = get_instance_attributes(self.rocket) | ||||||||||||||||
| rocket_simulation = RocketSimulation(**attributes) | ||||||||||||||||
| encoded_attributes = collect_attributes( | ||||||||||||||||
| self.rocket, | ||||||||||||||||
| [RocketSimulation, MotorSimulation] | ||||||||||||||||
| ) | ||||||||||||||||
| rocket_simulation = RocketSimulation(**encoded_attributes) | ||||||||||||||||
|
Comment on lines
+110
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Attribute backfill via collect_attributes is ineffective for a Rocket instance. The helper looks for obj.rocket / obj.rocket.motor; with a Rocket input this branch is skipped. Let rocketpy_encoder drive encoding directly, or adjust the utility to detect object shape. Apply this diff: - encoded_attributes = collect_attributes(
- self.rocket,
- [RocketSimulation, MotorSimulation]
- )
+ encoded_attributes = collect_attributes(self.rocket)If you prefer keeping class hints, update utils similarly to handle 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| return rocket_simulation | ||||||||||||||||
|
|
||||||||||||||||
| def get_rocket_binary(self) -> bytes: | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,24 @@ | ||||||
| from typing import Optional | ||||||
| from typing import Optional, Any | ||||||
| from datetime import datetime, timedelta | ||||||
| from pydantic import ConfigDict | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Datetime defaults are evaluated at import time; switch to default_factory. Avoid stale defaults in long-lived processes. Apply this diff: -from pydantic import ConfigDict
+from pydantic import ConfigDict, Field
@@
- date: Optional[Any] = datetime.today() + timedelta(days=1)
- local_date: Optional[Any] = datetime.today() + timedelta(days=1)
- datetime_date: Optional[Any] = datetime.today() + timedelta(days=1)
+ date: Optional[Any] = Field(default_factory=lambda: datetime.today() + timedelta(days=1))
+ local_date: Optional[Any] = Field(default_factory=lambda: datetime.today() + timedelta(days=1))
+ datetime_date: Optional[Any] = Field(default_factory=lambda: datetime.today() + timedelta(days=1))Also applies to: 38-40 🤖 Prompt for AI Agents |
||||||
| from src.views.interface import ApiBaseView | ||||||
| from src.models.environment import EnvironmentModel | ||||||
| from src.utils import AnyToPrimitive | ||||||
|
|
||||||
|
|
||||||
| class EnvironmentSimulation(ApiBaseView): | ||||||
| """ | ||||||
| Environment simulation view that handles dynamically encoded RocketPy Environment attributes. | ||||||
|
|
||||||
| Uses the new rocketpy_encoder which may return different attributes based on the | ||||||
| actual RocketPy Environment object. The model allows extra fields to accommodate | ||||||
| any new attributes that might be encoded. | ||||||
| """ | ||||||
|
|
||||||
| model_config = ConfigDict(extra='ignore', arbitrary_types_allowed=True) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra='ignore' contradicts the docstring; use extra='allow'. With extra='ignore', unknown encoded attributes are dropped instead of preserved. Apply this diff: - model_config = ConfigDict(extra='ignore', arbitrary_types_allowed=True)
+ model_config = ConfigDict(extra='allow', arbitrary_types_allowed=True)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| message: str = "Environment successfully simulated" | ||||||
|
|
||||||
| # Core Environment attributes (always present) | ||||||
| latitude: Optional[float] = None | ||||||
| longitude: Optional[float] = None | ||||||
| elevation: Optional[float] = 1 | ||||||
|
|
@@ -23,30 +35,32 @@ class EnvironmentSimulation(ApiBaseView): | |||||
| initial_hemisphere: Optional[str] = None | ||||||
| initial_ew: Optional[str] = None | ||||||
| max_expected_height: Optional[float] = None | ||||||
| date: Optional[datetime] = datetime.today() + timedelta(days=1) | ||||||
| local_date: Optional[datetime] = datetime.today() + timedelta(days=1) | ||||||
| datetime_date: Optional[datetime] = datetime.today() + timedelta(days=1) | ||||||
| ellipsoid: Optional[AnyToPrimitive] = None | ||||||
| barometric_height: Optional[AnyToPrimitive] = None | ||||||
| barometric_height_ISA: Optional[AnyToPrimitive] = None | ||||||
| pressure: Optional[AnyToPrimitive] = None | ||||||
| pressure_ISA: Optional[AnyToPrimitive] = None | ||||||
| temperature: Optional[AnyToPrimitive] = None | ||||||
| temperature_ISA: Optional[AnyToPrimitive] = None | ||||||
| density: Optional[AnyToPrimitive] = None | ||||||
| speed_of_sound: Optional[AnyToPrimitive] = None | ||||||
| dynamic_viscosity: Optional[AnyToPrimitive] = None | ||||||
| gravity: Optional[AnyToPrimitive] = None | ||||||
| somigliana_gravity: Optional[AnyToPrimitive] = None | ||||||
| wind_speed: Optional[AnyToPrimitive] = None | ||||||
| wind_direction: Optional[AnyToPrimitive] = None | ||||||
| wind_heading: Optional[AnyToPrimitive] = None | ||||||
| wind_velocity_x: Optional[AnyToPrimitive] = None | ||||||
| wind_velocity_y: Optional[AnyToPrimitive] = None | ||||||
| calculate_earth_radius: Optional[AnyToPrimitive] = None | ||||||
| decimal_degrees_to_arc_seconds: Optional[AnyToPrimitive] = None | ||||||
| geodesic_to_utm: Optional[AnyToPrimitive] = None | ||||||
| utm_to_geodesic: Optional[AnyToPrimitive] = None | ||||||
| date: Optional[Any] = datetime.today() + timedelta(days=1) | ||||||
| local_date: Optional[Any] = datetime.today() + timedelta(days=1) | ||||||
| datetime_date: Optional[Any] = datetime.today() + timedelta(days=1) | ||||||
|
|
||||||
| # Function attributes (discretized by rocketpy_encoder, serialized by RocketPyEncoder) | ||||||
| ellipsoid: Optional[Any] = None | ||||||
| barometric_height: Optional[Any] = None | ||||||
| barometric_height_ISA: Optional[Any] = None | ||||||
| pressure: Optional[Any] = None | ||||||
| pressure_ISA: Optional[Any] = None | ||||||
| temperature: Optional[Any] = None | ||||||
| temperature_ISA: Optional[Any] = None | ||||||
| density: Optional[Any] = None | ||||||
| speed_of_sound: Optional[Any] = None | ||||||
| dynamic_viscosity: Optional[Any] = None | ||||||
| gravity: Optional[Any] = None | ||||||
| somigliana_gravity: Optional[Any] = None | ||||||
| wind_speed: Optional[Any] = None | ||||||
| wind_direction: Optional[Any] = None | ||||||
| wind_heading: Optional[Any] = None | ||||||
| wind_velocity_x: Optional[Any] = None | ||||||
| wind_velocity_y: Optional[Any] = None | ||||||
| calculate_earth_radius: Optional[Any] = None | ||||||
| decimal_degrees_to_arc_seconds: Optional[Any] = None | ||||||
| geodesic_to_utm: Optional[Any] = None | ||||||
| utm_to_geodesic: Optional[Any] = None | ||||||
|
|
||||||
|
|
||||||
| class EnvironmentView(EnvironmentModel): | ||||||
|
|
||||||
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.
🛠️ Refactor suggestion
Passing EnvironmentSimulation to collect_attributes is a no-op for a bare Environment object.
collect_attributes tries obj.env for EnvironmentSimulation; with an Environment instance, that path is skipped. Rely on rocketpy_encoder directly or update the utility to handle both Flight and Environment inputs.
Apply this diff:
Optionally, harden collect_attributes to support both patterns:
🤖 Prompt for AI Agents