Skip to content

BUG: Fix the handling of reference pressure for older rpy files. #808

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

Merged
merged 2 commits into from
Apr 22, 2025

Conversation

phmbressan
Copy link
Collaborator

Pull request type

  • Code changes (bugfix, features)

Checklist

Current behavior

Currently, the introduction of net_thrust is the first addition of an attribute that impacts the behavior of the motor and flight classes. Therefore, the .rpy of version 1.8.0 does not have this parameters, which should be handled.

One notable point is that the tests are working as expected and caught this "file breaking change".

New behavior

This PR adds custom handling for the new attributes. Unfortunately, I did not find other ways beside manual handling (i.e. pinpointing in code the necessary attribute).

I am open to suggestions on how to deal with those parameter changes between older file versions.

Breaking change

  • Yes
  • No

@phmbressan phmbressan added the Bug Something isn't working label Apr 21, 2025
@phmbressan phmbressan added this to the Release v1.X.0 milestone Apr 21, 2025
@phmbressan phmbressan self-assigned this Apr 21, 2025
@phmbressan phmbressan requested a review from a team as a code owner April 21, 2025 12:28
@@ -518,7 +522,9 @@ def __evaluate_flight_outputs(self, flight, sim_idx):
) from e
outputs_dict = outputs_dict | additional_exports

return json.dumps(outputs_dict, cls=RocketPyEncoder) + "\n"
return (
json.dumps(outputs_dict, cls=RocketPyEncoder, **self._export_config) + "\n"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change had already been done by PR #679, however is was likely undone by merging errors at some point.

Copy link

codecov bot commented Apr 21, 2025

Codecov Report

Attention: Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.

Project coverage is 79.79%. Comparing base (4df0b38) to head (d512021).
Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
rocketpy/simulation/monte_carlo.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #808      +/-   ##
===========================================
+ Coverage    79.11%   79.79%   +0.67%     
===========================================
  Files           96       97       +1     
  Lines        11575    11899     +324     
===========================================
+ Hits          9158     9495     +337     
+ Misses        2417     2404      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-project-automation github-project-automation bot moved this from Backlog to Next Version in LibDev Roadmap Apr 21, 2025
@phmbressan phmbressan merged commit ee23bc5 into develop Apr 22, 2025
9 of 10 checks passed
@phmbressan phmbressan deleted the bug/encoding-fixes branch April 22, 2025 09:48
@github-project-automation github-project-automation bot moved this from Next Version to Closed in LibDev Roadmap Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants