BUG: Motor method 'export_eng' for liquid motors bug fix. (solves #473)#559
BUG: Motor method 'export_eng' for liquid motors bug fix. (solves #473)#559lucasfourier merged 5 commits intodevelopfrom
Conversation
|
I'd like to add that Issue #473 requests the implementation of tests. However, I'd like to request permission to not write tests for this method for now. My plan is to refactor the unit tests for the liquid_motor module later today, and I intend to include the tests for this method as part of that refactor. |
|
.eng files documentation: https://www.thrustcurve.org/info/raspformat.html Saving that for reference. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #559 +/- ##
===========================================
+ Coverage 72.47% 72.60% +0.12%
===========================================
Files 59 59
Lines 9567 9584 +17
===========================================
+ Hits 6934 6958 +24
+ Misses 2633 2626 -7 ☔ View full report in Codecov by Sentry. |
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
I know the code works, but I think there is a little code repetition that perhaps could be avoid, thus improving readability.
I will add a code suggestion here. I know it is also changing some other stuffs like the "with context" and adopting the f-strings instead of .format(), but I hope you can understand it. Feel free to accept the suggestions, while in this very same PR or in other.
def export_eng(self, file_name, motor_name):
"""Exports thrust curve data points and motor description to
.eng file format. A description of the format can be found
here: http://www.thrustcurve.org/raspformat.shtml
Parameters
----------
file_name : string
Name of the .eng file to be exported. E.g. 'test.eng'
motor_name : string
Name given to motor. Will appear in the description of the
.eng file. E.g. 'Mandioca'
Returns
-------
None
"""
# Open file
with open(file_name, "w") as file:
# Write first line
def get_attr_value(obj, attr_name, multiplier=1):
return multiplier * getattr(obj, attr_name, 0)
grain_outer_radius = get_attr_value(self, "grain_outer_radius", 2000)
grain_number = get_attr_value(self, "grain_number", 1000)
grain_initial_height = get_attr_value(self, "grain_initial_height")
grain_separation = get_attr_value(self, "grain_separation")
grain_total = grain_number * (grain_initial_height + grain_separation)
if grain_outer_radius == 0 or grain_total == 0:
warnings.warn(
"The motor object doesn't have some grain-related attributes. "
"Using zeros to write to file."
)
file.write(
f"{motor_name} {grain_outer_radius:3.1f} {grain_total:3.1f} 0 "
f"{self.propellant_initial_mass:2.3} "
f"{self.propellant_initial_mass:2.3} RocketPy\n"
)
# Write thrust curve data points
for time, thrust in self.thrust.source[1:-1, :]:
file.write(f"{time:.4f} {thrust:.3f}\n")
# Write last line
file.write(f"{self.thrust.source[-1, 0]:.4f} {0:.3f}\n")
return None|
@Gui-FernandesBR Much better, thanks. Committed your solution |
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Nice, well done @lucasfourier !!
Can you please update the CHANGELOG.md file with this PR?
It is important since it makes it easier for users and contributors to see precisely what changes have been made between each version of the project.
|
Done it |
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Thanks for updating the CHANGELOG, but it was wrong though.
Please add your PR to the [Unreleased] - yyyy-mm-dd section, under the Fixed.
The v1.2.0 (where you added) is already released.
|
Fixed it |
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Still requiring changes on the CHANGELOG. Please take a look at other sections of the file so you can keep the same pattern
There was a problem hiding this comment.
This is better:
## [Unreleased] - yyyy-mm-dd
<!-- These are the changes that were not release yet, please add them correctly.
Attention: The newest changes should be on top -->
### Added
### Changed
### Fixed
- BUG: export_eng 'Motor' method would not work for liquid motors. [#559](https://github.com/RocketPy-Team/RocketPy/pull/559)
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
As described in #473, 'export_eng' method does not work for liquid motors since it does not have grain related attributes.
New behavior
The Motor method 'export_eng' now works for liquid motors and the .eng file is created. For liquid motors, the first two parameters written to the file are now zeros, since those are grain related attributes.
Breaking change
Additional information
Below is the '.eng' file generated for the liquid motor defined in the SEB_liquid_motor.ipynb notebook.