-
-
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?
Changes from all commits
9339a43
fb23890
ba7c28b
7c7f805
6ecc358
5eab472
8450082
d1f9a97
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 |
---|---|---|
|
@@ -19,15 +19,17 @@ def __init__(self): | |
|
||
@repository_exception_handler | ||
async def create_motor(self, motor: MotorModel) -> str: | ||
return await self.insert(motor.model_dump()) | ||
return await self.insert(motor.model_dump(exclude_none=True)) | ||
|
||
@repository_exception_handler | ||
async def read_motor_by_id(self, motor_id: str) -> Optional[MotorModel]: | ||
return await self.find_by_id(data_id=motor_id) | ||
|
||
@repository_exception_handler | ||
async def update_motor_by_id(self, motor_id: str, motor: MotorModel): | ||
await self.update_by_id(motor.model_dump(), data_id=motor_id) | ||
await self.update_by_id( | ||
motor.model_dump(exclude_none=True), data_id=motor_id | ||
) | ||
Comment on lines
+30
to
+32
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. 💡 Verification agent 🧩 Analysis chainPUT behavior change: explicit nulls ignored on update Applying exclude_none=True here prevents clients from nulling fields. If unsetting is a supported use case, consider the $set/$unset approach mentioned for Rocket/Flight repositories; otherwise, update API documentation to make this explicit. 🏁 Script executed: #!/bin/bash
# Look for tests asserting null-able fields after update
rg -n "update_.*motor|client\.put\('/motors" -A 8 tests | rg -n "None|null" Length of output: 179 Explicit nulls are ignored on updates; choose to implement unset logic or update docs accordingly
🤖 Prompt for AI Agents
|
||
|
||
@repository_exception_handler | ||
async def delete_motor_by_id(self, motor_id: str): | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,48 +1,113 @@ | ||||||||||||||
# fork of https://github.com/encode/starlette/blob/master/starlette/middleware/gzip.py | ||||||||||||||
import gzip | ||||||||||||||
import io | ||||||||||||||
import logging | ||||||||||||||
import json | ||||||||||||||
import copy | ||||||||||||||
|
||||||||||||||
from typing import Annotated, NoReturn, Any | ||||||||||||||
import numpy as np | ||||||||||||||
from typing import NoReturn, Tuple | ||||||||||||||
|
||||||||||||||
from pydantic import PlainSerializer | ||||||||||||||
from rocketpy import Function | ||||||||||||||
from rocketpy._encoders import RocketPyEncoder | ||||||||||||||
from starlette.datastructures import Headers, MutableHeaders | ||||||||||||||
from starlette.types import ASGIApp, Message, Receive, Scope, Send | ||||||||||||||
|
||||||||||||||
logger = logging.getLogger(__name__) | ||||||||||||||
|
||||||||||||||
def to_python_primitive(v: Any) -> Any: | ||||||||||||||
|
||||||||||||||
class DiscretizeConfig: | ||||||||||||||
""" | ||||||||||||||
Convert complex types to Python primitives. | ||||||||||||||
Configuration class for RocketPy function discretization. | ||||||||||||||
|
||||||||||||||
Args: | ||||||||||||||
v: Any value, particularly those with a 'source' attribute | ||||||||||||||
containing numpy arrays or generic types. | ||||||||||||||
This class allows easy configuration of discretization parameters | ||||||||||||||
for different types of RocketPy objects and their callable attributes. | ||||||||||||||
""" | ||||||||||||||
|
||||||||||||||
Returns: | ||||||||||||||
The primitive representation of the input value. | ||||||||||||||
def __init__( | ||||||||||||||
self, bounds: Tuple[float, float] = (0, 10), samples: int = 200 | ||||||||||||||
): | ||||||||||||||
self.bounds = bounds | ||||||||||||||
self.samples = samples | ||||||||||||||
|
||||||||||||||
@classmethod | ||||||||||||||
def for_environment(cls) -> 'DiscretizeConfig': | ||||||||||||||
return cls(bounds=(0, 50000), samples=100) | ||||||||||||||
|
||||||||||||||
@classmethod | ||||||||||||||
def for_motor(cls) -> 'DiscretizeConfig': | ||||||||||||||
return cls(bounds=(0, 10), samples=150) | ||||||||||||||
|
||||||||||||||
@classmethod | ||||||||||||||
def for_rocket(cls) -> 'DiscretizeConfig': | ||||||||||||||
return cls(bounds=(0, 1), samples=100) | ||||||||||||||
|
||||||||||||||
@classmethod | ||||||||||||||
def for_flight(cls) -> 'DiscretizeConfig': | ||||||||||||||
return cls(bounds=(0, 30), samples=200) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
def rocketpy_encoder(obj, config: DiscretizeConfig = DiscretizeConfig()): | ||||||||||||||
GabrielBarberini marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
""" | ||||||||||||||
if hasattr(v, "source"): | ||||||||||||||
if isinstance(v.source, np.ndarray): | ||||||||||||||
return v.source.tolist() | ||||||||||||||
Encode a RocketPy object using official RocketPy encoders. | ||||||||||||||
|
||||||||||||||
if isinstance(v.source, (np.generic,)): | ||||||||||||||
return v.source.item() | ||||||||||||||
This function creates a copy of the object, discretizes callable Function | ||||||||||||||
attributes on the copy, and then uses RocketPy's official RocketPyEncoder for | ||||||||||||||
complete object serialization. The original object remains unchanged. | ||||||||||||||
|
||||||||||||||
return str(v.source) | ||||||||||||||
Args: | ||||||||||||||
obj: RocketPy object (Environment, Motor, Rocket, Flight) | ||||||||||||||
config: DiscretizeConfig object with discretization parameters (optional) | ||||||||||||||
|
||||||||||||||
if isinstance(v, (np.generic,)): | ||||||||||||||
return v.item() | ||||||||||||||
Returns: | ||||||||||||||
Dictionary of encoded attributes | ||||||||||||||
""" | ||||||||||||||
|
||||||||||||||
if isinstance(v, (np.ndarray,)): | ||||||||||||||
return v.tolist() | ||||||||||||||
# Create a copy to avoid mutating the original object | ||||||||||||||
obj_copy = copy.deepcopy(obj) | ||||||||||||||
|
||||||||||||||
for attr_name in dir(obj_copy): | ||||||||||||||
if attr_name.startswith('_'): | ||||||||||||||
continue | ||||||||||||||
|
||||||||||||||
try: | ||||||||||||||
attr_value = getattr(obj_copy, attr_name) | ||||||||||||||
except Exception: | ||||||||||||||
continue | ||||||||||||||
|
||||||||||||||
if callable(attr_value) and isinstance(attr_value, Function): | ||||||||||||||
try: | ||||||||||||||
discretized_func = Function(attr_value.source) | ||||||||||||||
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. Creating a new Function from
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||
discretized_func.set_discrete( | ||||||||||||||
lower=config.bounds[0], | ||||||||||||||
upper=config.bounds[1], | ||||||||||||||
samples=config.samples, | ||||||||||||||
mutate_self=True, | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
return str(v) | ||||||||||||||
setattr(obj_copy, attr_name, discretized_func) | ||||||||||||||
|
||||||||||||||
except Exception as e: | ||||||||||||||
logger.warning(f"Failed to discretize {attr_name}: {e}") | ||||||||||||||
|
||||||||||||||
AnyToPrimitive = Annotated[ | ||||||||||||||
Any, | ||||||||||||||
PlainSerializer(to_python_primitive), | ||||||||||||||
] | ||||||||||||||
try: | ||||||||||||||
json_str = json.dumps( | ||||||||||||||
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. @phmbressan @GabrielBarberini not sure who this is for but when i run the env class simulation I get an incorrect date format here
get_instance_attributes parses it as a natural datetime object. Is this a encoder issue? Observed this behavior while printing this |
||||||||||||||
obj_copy, | ||||||||||||||
cls=RocketPyEncoder, | ||||||||||||||
include_outputs=True, | ||||||||||||||
include_function_data=True, | ||||||||||||||
) | ||||||||||||||
return json.loads(json_str) | ||||||||||||||
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. The encoding process involves creating a JSON string and then immediately parsing it back to a dictionary. Consider using the encoder directly to return a dictionary without the JSON round-trip if possible.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||
except Exception as e: | ||||||||||||||
logger.warning(f"Failed to encode with RocketPyEncoder: {e}") | ||||||||||||||
attributes = {} | ||||||||||||||
for attr_name in dir(obj_copy): | ||||||||||||||
if not attr_name.startswith('_'): | ||||||||||||||
try: | ||||||||||||||
attr_value = getattr(obj_copy, attr_name) | ||||||||||||||
if not callable(attr_value): | ||||||||||||||
attributes[attr_name] = str(attr_value) | ||||||||||||||
GabrielBarberini marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
except Exception: | ||||||||||||||
continue | ||||||||||||||
return attributes | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
class RocketPyGZipMiddleware: | ||||||||||||||
|
@@ -70,6 +135,7 @@ async def __call__( | |||||||||||||
|
||||||||||||||
|
||||||||||||||
class GZipResponder: | ||||||||||||||
# fork of https://github.com/encode/starlette/blob/master/starlette/middleware/gzip.py | ||||||||||||||
def __init__( | ||||||||||||||
self, app: ASGIApp, minimum_size: int, compresslevel: int = 9 | ||||||||||||||
) -> None: | ||||||||||||||
|
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.
💡 Verification agent
🧩 Analysis chain
Update semantics changed: None no longer clears fields
Same concern as in RocketRepository: exclude_none=True will ignore fields explicitly set to None, preventing clearing. Decide whether:
Happy to draft an interface-level change to support $unset cleanly if desired.
🏁 Script executed:
Length of output: 650
Support clearing fields set to None via $unset
Currently, all repository update calls use
model_dump(exclude_none=True)
, so any fields explicitly set toNone
are omitted and won’t be cleared in the database. Please decide on one of the following:• If omitting
None
is the intended behavior, add clear documentation (both inline and in our API docs) stating that fields cannot be cleared viaNone
.• Otherwise, extend
update_by_id
to handle$unset
:– Build a
$set
payload from non-None
values– Build an
$unset
payload from keys withNone
values– Send both in a single update operation
Apply the chosen approach consistently in:
Let me know if you’d like a draft for the interface-level refactor to support
$unset
.