MNT: adapt discretization schema to RocketPy encoder. #52
Conversation
Co-authored-by: Copilot <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces legacy attribute extraction with collect_attributes and rocketpy_encoder across services, shifts Simulation view models to dynamic ApiBaseView-based schemas, broadens Environment/Flight/Motor/Rocket models to accept encoder-driven fields, updates a rocket model field type (int→float), and adjusts middleware default. No public service method signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Service as *Service (Flight/ Rocket/ Motor/ Env)*
participant Utils as collect_attributes
participant Encoder as rocketpy_encoder / RocketPyEncoder
participant View as *Simulation Model*
Client->>Service: get_*_simulation()
Service->>Utils: collect_attributes(obj, [Sim classes])
Utils->>Encoder: encode aggregated attributes
Encoder-->>Utils: primitive dict (encoded_attributes)
Utils-->>Service: encoded_attributes
Service->>View: Simulation(**encoded_attributes)
View-->>Client: Simulation payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
src/utils.py
Outdated
| attr_value = getattr(obj_copy, attr_name) | ||
| except Exception: | ||
| continue | ||
| def collect_simulation_attributes(flight_obj, flight_simulation_class, rocket_simulation_class, motor_simulation_class, environment_simulation_class): |
There was a problem hiding this comment.
I would rename flight_obj to a more generic name, since this function is used elsewhere in the code with other objects (e.g. rockets, environments etc).
Besides that, it seems that at points this function's parameters are used in the wrong order: e.g. a MotorSimulation being passed as the flight_simulatiom_class. I see two ways of making this a bit more safe:
- letting the function dispatch the correct handling:
def collect_attributes(obj, attribute_classes=None):
if attribute_classes is None:
attribute_classes = [] # Default empty list
for attribute_class in attribute_classes:
if issubclass(attribute_class, MotorSimulation):
# for loop for the motor attr
elif issubclass(attribute_class, RocketSimulation):
# ...When using it, things would work like:
collect_attributes(self.rocket, [RocketSimulation, MotorSimulation])- maybe the other attributes could be made into a **kwargs, so that things would work like:
def collect_attributes(obj, **kwargs):
motor_simulation_class = kwargs.get("motor_simulation_class", None)
... # All other parameters of other simulation classes
if motor_simulation_class is not None:
# for loop for getting the motor attr
... # Other for loopsWhen calling it, things would be similar:
collect_attributes(self.motor, motor_simulation_class=MotorSimulation)I have a slight preference for the first way, but whatever fits and works better should do it.
src/utils.py
Outdated
| value = getattr(flight_obj, key) | ||
| attributes[key] = value |
There was a problem hiding this comment.
I believe that if the attribute, i.e., the key is already in the attributes, you can skip it. That should save you some time if an attribute that is listed in the models is already included by default in rocketpy encoding. For instance:
if not key in attributes:
attributes[key] = getattr(flight_obj, key)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
GabrielBarberini
changed the base branch from
rocketpy_encoders_refactor
to
master
There was a problem hiding this comment.
Pull Request Overview
This PR adapts the discretization schema to use RocketPy's official encoder system for serializing RocketPy objects. The changes replace a custom AnyToPrimitive utility with RocketPy's built-in RocketPyEncoder and restructure the simulation view classes to better handle dynamic attributes from encoded RocketPy objects.
- Replaces custom serialization with RocketPy's official
RocketPyEncoderandrocketpy_encoderfunction - Updates simulation view classes to inherit from
ApiBaseViewinstead of each other and use dynamic attribute handling - Reorganizes attributes in simulation classes with clearer grouping and comments
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/views/rocket.py | Updates RocketSimulation to use ApiBaseView, reorganizes attributes, and adds dynamic encoding support |
| src/views/motor.py | Restructures MotorSimulation with better attribute organization and dynamic encoding |
| src/views/flight.py | Changes FlightSimulation inheritance and reorganizes flight-specific attributes |
| src/views/environment.py | Updates EnvironmentSimulation with new encoding approach and attribute grouping |
| src/utils.py | Replaces AnyToPrimitive with rocketpy_encoder and adds collect_simulation_attributes function |
| src/services/*.py | Updates service classes to use new collect_simulation_attributes function |
| src/models/rocket.py | Fixes type annotation for center_of_mass_without_motor |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/services/rocket.py
Outdated
| RocketSimulation, | ||
| MotorSimulation, |
There was a problem hiding this comment.
The collect_simulation_attributes function expects 5 parameters but only 3 are provided. According to the function signature in utils.py, it needs flight_obj, flight_simulation_class, rocket_simulation_class, motor_simulation_class, and environment_simulation_class.
| RocketSimulation, | |
| MotorSimulation, | |
| RocketSimulation, | |
| MotorSimulation, | |
| EnvironmentSimulation, |
src/services/motor.py
Outdated
| self.motor, | ||
| MotorSimulation, |
There was a problem hiding this comment.
The collect_simulation_attributes function expects 5 parameters but only 2 are provided. According to the function signature in utils.py, it needs flight_obj, flight_simulation_class, rocket_simulation_class, motor_simulation_class, and environment_simulation_class.
| self.motor, | |
| MotorSimulation, | |
| self.motor, | |
| None, # flight_simulation_class | |
| None, # rocket_simulation_class | |
| MotorSimulation, | |
| None, # environment_simulation_class |
src/services/environment.py
Outdated
| encoded_attributes = collect_simulation_attributes( | ||
| self.environment, | ||
| EnvironmentSimulation, |
There was a problem hiding this comment.
The collect_simulation_attributes function expects 5 parameters but only 2 are provided. According to the function signature in utils.py, it needs flight_obj, flight_simulation_class, rocket_simulation_class, motor_simulation_class, and environment_simulation_class.
| encoded_attributes = collect_simulation_attributes( | |
| self.environment, | |
| EnvironmentSimulation, | |
| None, # flight_simulation_class | |
| None, # rocket_simulation_class | |
| None, # motor_simulation_class | |
| EnvironmentSimulation, # environment_simulation_class |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils.py (1)
212-215: Content-Type check for octet-stream is incorrectYou’re matching header values without verifying the header name, which can give false positives/negatives. Check the
Content-Typeheader explicitly and allow for parameters.- if ((len(body) < self.minimum_size) and not more_body) or any( - value == b'application/octet-stream' - for header, value in self.initial_message["headers"] - ): + if ((len(body) < self.minimum_size) and not more_body) or any( + header.lower() == b"content-type" and value.startswith(b"application/octet-stream") + for header, value in self.initial_message["headers"] + ):
🧹 Nitpick comments (6)
src/views/environment.py (1)
24-24: Align elevation default with EnvironmentModel for consistency.EnvironmentModel uses 0.0; here it’s 1. Consider aligning to avoid surprises across views.
Apply this diff:
- elevation: Optional[float] = 1 + elevation: Optional[float] = 0.0src/services/motor.py (2)
47-48: Redundant boolean expression
False or motor.reshape_thrust_curveis equivalent tomotor.reshape_thrust_curve. Simplify.- "reshape_thrust_curve": False or motor.reshape_thrust_curve, + "reshape_thrust_curve": motor.reshape_thrust_curve,
50-51: Unnecessary enum wrapping in match
motor.motor_kindis already aMotorKinds. Remove the extra cast to avoid redundant work.- match MotorKinds(motor.motor_kind): + match motor.motor_kind:src/utils.py (3)
45-55: Docstring: broaden object type wording
collect_attributesworks for Flight, Rocket, Motor, and Environment. Update to avoid confusion.- Collect attributes from various simulation classes and populate them from the flight object. + Collect attributes from various simulation classes and populate them from the object. @@ - obj: RocketPy Flight object + obj: RocketPy object (Flight, Rocket, Motor, or Environment)
61-140: Reduce nesting and avoid swallowing exceptions silentlyThere are multiple nested blocks and broad
except Exception: passstatements, which Pylint is flagging and which can hide real bugs. Replace the broad catches with debug logs and consider flattening the loops.Minimal safer change (apply to each section):
- except Exception: - pass + except Exception as e: + logger.debug("collect_attributes: flight attr '%s' not copied: %s", key, e) @@ - except Exception: - pass + except Exception as e: + logger.debug("collect_attributes: rocket attr '%s' not copied: %s", key, e) @@ - except Exception: - pass + except Exception as e: + logger.debug("collect_attributes: motor attr '%s' not copied: %s", key, e) @@ - except Exception: - pass + except Exception as e: + logger.debug("collect_attributes: env attr '%s' not copied: %s", key, e)Follow-up (optional): extract a small helper to DRY nested set-logic and cut branches.
1-266: Split middleware out of utils to reduce coupling and complexity
src/utils.pycurrently mixes encoding helpers with ASGI middleware. This increases module complexity and risks cyclical imports. MoveRocketPyGZipMiddleware/GZipRespondertosrc/middleware/gzip.pyand import where needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
src/models/rocket.py(1 hunks)src/services/environment.py(2 hunks)src/services/flight.py(2 hunks)src/services/motor.py(2 hunks)src/services/rocket.py(2 hunks)src/utils.py(2 hunks)src/views/environment.py(2 hunks)src/views/flight.py(1 hunks)src/views/motor.py(1 hunks)src/views/rocket.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-15T15:12:21.314Z
Learnt from: GabrielBarberini
PR: RocketPy-Team/Infinity-API#38
File: lib/routes/motor.py:75-75
Timestamp: 2024-11-15T15:12:21.314Z
Learning: When modifying API route paths in `lib/routes/motor.py`, changing from `/rocketpy/{motor_id}` to `/{motor_id}/rocketpy` is acceptable when no external consumers are affected. It's acceptable to introduce this breaking change if the team has decided to adopt the new approach, provided that the `README` and related documentation are updated accordingly.
Applied to files:
src/views/rocket.pysrc/views/motor.py
🧬 Code graph analysis (10)
src/models/rocket.py (3)
src/routes/rocket.py (1)
update_rocket(56-68)src/repositories/rocket.py (1)
update_rocket_by_id(29-32)src/routes/flight.py (1)
update_flight_rocket(140-155)
src/services/flight.py (4)
src/views/rocket.py (1)
RocketSimulation(8-49)src/views/motor.py (1)
MotorSimulation(7-67)src/views/environment.py (1)
EnvironmentSimulation(8-63)src/utils.py (1)
collect_attributes(45-144)
src/services/environment.py (3)
src/utils.py (1)
collect_attributes(45-144)src/views/environment.py (1)
EnvironmentSimulation(8-63)src/controllers/environment.py (2)
get_environment_simulation(44-61)EnvironmentController(10-61)
src/services/rocket.py (3)
src/views/motor.py (1)
MotorSimulation(7-67)src/utils.py (1)
collect_attributes(45-144)src/views/rocket.py (1)
RocketSimulation(8-49)
src/services/motor.py (2)
src/utils.py (1)
collect_attributes(45-144)src/views/motor.py (1)
MotorSimulation(7-67)
src/utils.py (5)
src/views/environment.py (1)
EnvironmentSimulation(8-63)src/views/flight.py (1)
FlightSimulation(9-136)src/views/motor.py (1)
MotorSimulation(7-67)src/views/rocket.py (1)
RocketSimulation(8-49)tests/unit/test_routes/test_rockets_route.py (1)
stub_rocket_simulation_dump(24-27)
src/views/rocket.py (4)
src/services/rocket.py (3)
rocket(96-97)rocket(100-101)check_parachute_trigger(221-236)src/models/rocket.py (1)
RocketModel(13-63)src/views/interface.py (1)
ApiBaseView(4-5)src/views/motor.py (1)
MotorSimulation(7-67)
src/views/motor.py (3)
src/views/interface.py (1)
ApiBaseView(4-5)src/services/motor.py (2)
motor(129-130)motor(133-134)src/models/motor.py (1)
MotorModel(16-95)
src/views/environment.py (3)
src/views/interface.py (1)
ApiBaseView(4-5)src/models/environment.py (2)
EnvironmentModel(6-54)EnvironmentModel(12-58)src/controllers/environment.py (1)
EnvironmentController(10-61)
src/views/flight.py (5)
src/services/flight.py (2)
flight(46-47)flight(50-51)src/models/flight.py (1)
FlightModel(7-71)src/views/interface.py (1)
ApiBaseView(4-5)src/views/rocket.py (1)
RocketSimulation(8-49)src/views/environment.py (1)
EnvironmentSimulation(8-63)
🪛 GitHub Actions: Pylint
src/utils.py
[warning] 64-64: Trailing whitespace (trailing-whitespace) [C0303]
[warning] 79-79: Trailing whitespace (trailing-whitespace) [C0303]
[warning] 82-82: Trailing whitespace (trailing-whitespace) [C0303]
[warning] 99-99: Trailing whitespace (trailing-whitespace) [C0303]
[warning] 102-102: Trailing whitespace (trailing-whitespace) [C0303]
[warning] 121-121: Trailing whitespace (trailing-whitespace) [C0303]
[warning] 61-61: Too many nested blocks (6/5) (too-many-nested-blocks)
[warning] 61-61: Too many nested blocks (7/5) (too-many-nested-blocks)
[warning] 61-61: Too many nested blocks (7/5) (too-many-nested-blocks)
[warning] 45-45: Too many branches (31/12) (too-many-branches)
[warning] 45-45: Too many statements (70/50) (too-many-statements)
[warning] 13-13: Import order violation: third-party imports should be placed before first-party imports (C0411)
[warning] 14-14: Import order violation: starlette.datastructures.Headers should be placed before first-party imports (C0411)
[warning] 15-15: Import order violation: starlette.types.ASGIApp should be placed before first-party imports (C0411)
src/views/rocket.py
[warning] 1-1: Unused import 'List' from typing (W0611)
[warning] 1-1: Unused import 'Tuple' from typing (W0611)
🪛 Ruff (0.12.2)
src/views/rocket.py
1-1: typing.List imported but unused
Remove unused import
(F401)
1-1: typing.Tuple imported but unused
Remove unused import
(F401)
🔇 Additional comments (6)
src/models/rocket.py (1)
22-22: Type change to float looks correct; check downstream persistence/clients.This aligns with RocketSimulation (Optional[float]) and RocketPy expectations. Please verify stored documents and any clients sending integers still deserialize to float as intended.
src/services/flight.py (1)
60-65: LGTM: unified encoder path for Flight, Rocket, Motor, Env.This is the correct place to provide all attribute classes; collect_attributes matches the Flight object shape here.
src/services/motor.py (1)
143-148: Switch to collect_attributes + MotorSimulation construction looks goodThe dynamic-encoding integration is correct and aligns with the new encoder-driven schema.
src/utils.py (1)
8-15: Reorder imports and addsrc.prefix
Move third-party imports above first-party and update local imports with thesrc.prefix:-from views.environment import EnvironmentSimulation -from views.flight import FlightSimulation -from views.motor import MotorSimulation -from views.rocket import RocketSimulation - -from rocketpy._encoders import RocketPyEncoder -from starlette.datastructures import Headers, MutableHeaders -from starlette.types import ASGIApp, Message, Receive, Scope, Send +from rocketpy._encoders import RocketPyEncoder +from starlette.datastructures import Headers, MutableHeaders +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +from src.views.environment import EnvironmentSimulation +from src.views.flight import FlightSimulation +from src.views.motor import MotorSimulation +from src.views.rocket import RocketSimulationVerify in your environment that imports resolve at runtime and lint warnings are cleared.
src/views/motor.py (1)
7-17: Dynamic MotorSimulation schema LGTMModel config and optional fields align with encoder-driven outputs and will ignore unknowns safely.
src/views/rocket.py (1)
1-2: Remove unused imports
ListandTupleare unused insrc/views/rocket.pyand trigger F401; remove them.-from typing import Optional, Any, List, Tuple +from typing import Optional, Any
| encoded_attributes = collect_attributes( | ||
| self.environment, | ||
| [EnvironmentSimulation], | ||
| ) | ||
| env_simulation = EnvironmentSimulation(**encoded_attributes) | ||
| return env_simulation |
There was a problem hiding this comment.
🛠️ 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:
- encoded_attributes = collect_attributes(
- self.environment,
- [EnvironmentSimulation],
- )
+ encoded_attributes = collect_attributes(self.environment)Optionally, harden collect_attributes to support both patterns:
# utils.py (illustrative)
elif issubclass(attribute_class, EnvironmentSimulation):
environment_attributes_list = [...]
try:
src = getattr(obj, "env", obj) # works for Flight or Environment
for key in environment_attributes_list:
if key not in attributes.get("env", {}):
try:
value = getattr(src, key)
if "env" not in attributes and src is not obj:
attributes["env"] = {}
(attributes["env"] if src is not obj else attributes)[key] = value
except AttributeError:
pass
except Exception:
pass🤖 Prompt for AI Agents
In src/services/environment.py around lines 57 to 62, passing
EnvironmentSimulation as the attribute_class into collect_attributes is a no-op
for a bare Environment instance because collect_attributes looks up obj.env for
EnvironmentSimulation and skips when obj is an Environment; fix by either (A)
bypassing collect_attributes and encode the environment directly with
rocketpy_encoder (i.e., call the encoder on self.environment and construct
EnvironmentSimulation from that result), or (B) update the collect_attributes
utility to handle both Flight and Environment by looking up src = getattr(obj,
"env", obj) before extracting environment-specific keys so attributes are
populated whether the caller is a Flight or an Environment object.
| encoded_attributes = collect_attributes( | ||
| self.rocket, | ||
| [RocketSimulation, MotorSimulation] | ||
| ) | ||
| rocket_simulation = RocketSimulation(**encoded_attributes) |
There was a problem hiding this comment.
🛠️ 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 src = getattr(obj, "rocket", obj) and src = getattr(getattr(obj, "rocket", obj), "motor", getattr(obj, "motor", None)).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| encoded_attributes = collect_attributes( | |
| self.rocket, | |
| [RocketSimulation, MotorSimulation] | |
| ) | |
| rocket_simulation = RocketSimulation(**encoded_attributes) | |
| encoded_attributes = collect_attributes(self.rocket) | |
| rocket_simulation = RocketSimulation(**encoded_attributes) |
🤖 Prompt for AI Agents
In src/services/rocket.py around lines 110 to 114, collect_attributes is
currently ineffective for a Rocket instance because it expects obj.rocket /
obj.rocket.motor and skips when passed a Rocket; change the call to let
rocketpy_encoder handle encoding directly or update the utility to detect shape:
when implementing the fix, either (A) pass the original Rocket to
rocketpy_encoder instead of using collect_attributes so encoding is driven by
the encoder, or (B) modify collect_attributes to use src = getattr(obj,
"rocket", obj) and motor_src = getattr(src, "motor", getattr(obj, "motor",
None)) so it correctly backfills attributes for both wrapper and direct Rocket
instances, preserving existing class hints.
| from typing import Optional | ||
| from typing import Optional, Any | ||
| from datetime import datetime, timedelta | ||
| from pydantic import ConfigDict |
There was a problem hiding this comment.
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
In src/views/environment.py lines 3 and 38-40, datetime defaults are being set
at import time; replace any model/field defaults like default=datetime.now(...)
or default=datetime.utcnow(...) with a default_factory so the timestamp is
evaluated when an instance is created. Import and use pydantic Field (or the
appropriate model field API) with default_factory=datetime.now/utcnow for those
fields, or supply a callable that returns the current datetime, ensuring new
instances get fresh timestamps instead of stale import-time values.
| """ | ||
|
|
||
| model_config = ConfigDict(extra='ignore', arbitrary_types_allowed=True) | ||
|
|
There was a problem hiding this comment.
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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| model_config = ConfigDict(extra='ignore', arbitrary_types_allowed=True) | |
| model_config = ConfigDict(extra='allow', arbitrary_types_allowed=True) |
🤖 Prompt for AI Agents
In src/views/environment.py around line 17, the ConfigDict is set to
extra='ignore' which contradicts the docstring; change the model_config to use
extra='allow' (i.e., model_config = ConfigDict(extra='allow',
arbitrary_types_allowed=True)) so unknown encoded attributes are preserved
instead of dropped; update the single assignment on that line accordingly.
| Inherits from both RocketSimulation and EnvironmentSimulation, and adds flight-specific | ||
| attributes. Uses the new rocketpy_encoder which may return different attributes based | ||
| on the actual RocketPy Flight object. The model allows extra fields to accommodate | ||
| any new attributes that might be encoded. | ||
| """ |
There was a problem hiding this comment.
Docstring contradiction: composition vs inheritance
FlightSimulation no longer inherits from RocketSimulation/EnvironmentSimulation; it composes them via rocket and env. Update docstring.
- Inherits from both RocketSimulation and EnvironmentSimulation, and adds flight-specific
+ Composes RocketSimulation and EnvironmentSimulation via the "rocket" and "env" fields, and adds flight-specific📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Inherits from both RocketSimulation and EnvironmentSimulation, and adds flight-specific | |
| attributes. Uses the new rocketpy_encoder which may return different attributes based | |
| on the actual RocketPy Flight object. The model allows extra fields to accommodate | |
| any new attributes that might be encoded. | |
| """ | |
| """ | |
| Composes RocketSimulation and EnvironmentSimulation via the "rocket" and "env" fields, and adds flight-specific | |
| attributes. Uses the new rocketpy_encoder which may return different attributes based | |
| on the actual RocketPy Flight object. The model allows extra fields to accommodate | |
| any new attributes that might be encoded. | |
| """ |
🤖 Prompt for AI Agents
In src/views/flight.py around lines 13 to 17, the docstring incorrectly states
that FlightSimulation inherits from RocketSimulation and EnvironmentSimulation;
update it to say FlightSimulation composes those via its rocket and env
attributes (e.g., holds RocketSimulation and EnvironmentSimulation instances),
remove any wording about inheritance, and keep the notes about flight-specific
attributes, use of the new rocketpy_encoder which may add different attributes,
and allowing extra fields to accommodate encoded attributes.
| Inherits from MotorSimulation and adds rocket-specific attributes. Uses the new | ||
| rocketpy_encoder which may return different attributes based on the actual | ||
| RocketPy Rocket object. The model allows extra fields to accommodate any new | ||
| attributes that might be encoded. | ||
| """ |
There was a problem hiding this comment.
Docstring contradiction: the class does not inherit from MotorSimulation
It composes MotorSimulation via the motor field. Update the wording to avoid misleading readers.
- Inherits from MotorSimulation and adds rocket-specific attributes. Uses the new
+ Composes MotorSimulation via the "motor" field and adds rocket-specific attributes. Uses the new📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Inherits from MotorSimulation and adds rocket-specific attributes. Uses the new | |
| rocketpy_encoder which may return different attributes based on the actual | |
| RocketPy Rocket object. The model allows extra fields to accommodate any new | |
| attributes that might be encoded. | |
| """ | |
| Composes MotorSimulation via the "motor" field and adds rocket-specific attributes. Uses the new | |
| rocketpy_encoder which may return different attributes based on the actual | |
| RocketPy Rocket object. The model allows extra fields to accommodate any new | |
| attributes that might be encoded. | |
| """ |
🤖 Prompt for AI Agents
In src/views/rocket.py around lines 12 to 16, the class docstring incorrectly
states it "Inherits from MotorSimulation" while the class actually composes
MotorSimulation via a motor field; update the docstring to state that it
composes or contains a MotorSimulation instance (e.g., "composes MotorSimulation
via the motor field") and adjust any phrasing that implies inheritance so the
description accurately reflects composition and the use of rocketpy_encoder and
extra fields.
|
discussion moved to #58 |
This PR is still in draft state, pending the soon to be done discussion on the discrete encoding branch from the RocketPy repository.
Summary by CodeRabbit
New Features
Refactor
Changes