-
-
Notifications
You must be signed in to change notification settings - Fork 197
ENH: Discretized and No-Pickle Encoding Options #827
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: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #827 +/- ##
===========================================
+ Coverage 80.02% 80.11% +0.09%
===========================================
Files 98 98
Lines 12004 12115 +111
===========================================
+ Hits 9606 9706 +100
- Misses 2398 2409 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 pull request introduces new encoding options—discretization and no‐pickle encoding—for RocketPy objects by refactoring the to_dict methods across many modules to accept keyword arguments (including include_outputs, discretize, and pickle_callables) instead of a fixed boolean flag. The changes affect multiple areas of the codebase including rocket components, aero surfaces, motors, math utilities, environment modules, and the custom JSON encoder.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
rocketpy/rocket/components.py | Updated to_dict signature to accept **kwargs, removing the fixed include_outputs parameter. |
rocketpy/rocket/aero_surface/*.py | Refactored to_dict implementations within tail, rail_buttons, nose_cone, and fins modules to support new kwargs for discretization. |
rocketpy/motors/*.py | Updated to_dict methods in tank_geometry, tank, solid_motor, motor, liquid_motor, and hybrid_motor to use **kwargs with discretize handling. |
rocketpy/motors/fluid.py | Modified to_dict signature to accept **kwargs. |
rocketpy/mathutils/vector_matrix.py | Updated to_dict method to use **kwargs instead of a fixed include_outputs flag. |
rocketpy/mathutils/function.py | Refactored to_dict to incorporate discretize and pickle_callables flags and updated related logic. |
rocketpy/environment/environment.py | Added discretize handling for environmental data in to_dict. |
rocketpy/_encoders.py | Modified the custom JSON encoder to pass the updated kwargs to to_dict. |
CHANGELOG.md | Documented the enhancement with an entry for "Discretized and No-Pickle Encoding Options". |
Comments suppressed due to low confidence (2)
rocketpy/_encoders.py:50
- Consider updating the initializer docstring to clearly document the new 'discretize' and 'pickle_callables' kwargs, so users know how to control encoding behavior.
self.pickle_callables = kwargs.pop("pickle_callables", True)
rocketpy/motors/motor.py:1273
- Ensure that the updated kwargs usage (including 'discretize' and 'include_outputs') is accurately reflected in the method docstring and accompanying tests, to avoid potential confusion with legacy behavior.
if kwargs.get("discretize", False):
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.
LGTM
Pull request type
Checklist
black rocketpy/ tests/
) has passed locallypytest tests -m slow --runslow
) have passed locallyCHANGELOG.md
has been updatedCurrent behavior
The standard
RocketPyEncoder
currently does not support custom options for users/clients that may have the following needs:callable
definedFunctions
written out in a human readable format. Of course, this has the drawback that the originalcallable
cannot be exactly reproduced on decoding;New behavior
The added options
discretize
andallow_pickle
work as described above. The biggest challenge of the PR (which required a bit of hard coding) was setting discretization bounds for some of RocketPy callableFunctions
.Breaking change