-
-
Notifications
You must be signed in to change notification settings - Fork 176
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
ENH: Eliminating multiple plots for inertia components #566
ENH: Eliminating multiple plots for inertia components #566
Conversation
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.
The submitted code in this PR doesn't seem to be working.
@dipesh2212 we should work on
Here are some comments to guide you:
- Try to read @juliomachad0 's solution, you might benefit from his approach in this case.
- The lines like
self.I_11(*self.motor.burn_time)
in the motor_plots.py file will no longer work, please update the_MotorPlots.all
method. - Always run the tests before submitting your code, this prevents you from submitting a code that is not working.
- I think you could easily use the
Function.compare_plots
to easily create a single figure with all the inertia Functions and then display them.
Also, some other "small but important" things that I noticed:
- Rename your PR to start with
ENH:
, thus showing clearly the intention of your PR. - Please run black before submitting your code. (see the Makefile in the root directory for more details)
- If possible, please also provides screenshot of the plots you have modified, this allows us to understand what is the expected behavior without needing to run the code on our own.
4a9912b
to
81e92c6
Compare
…ttps://github.com/RocketPy-Team/RocketPy into 527-eliminating-multiple-plots-for-inertia_tensors
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #566 +/- ##
===========================================
- Coverage 71.92% 71.86% -0.07%
===========================================
Files 70 70
Lines 10334 10320 -14
===========================================
- Hits 7433 7416 -17
- Misses 2901 2904 +3 ☔ View full report in Codecov by Sentry. |
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.
This solves the issue!
Pull request type
Checklist
Current behavior
There are multiple inertial tensor plots that are being plotted, it is kinda annoying to deal with those
New behavior
I've tried to integrate all the plots in one single plot which can compare the inertia tensor in a convenient way
Breaking change