Skip to content

BUG: fin_flutter_analysis doesn't find any fin set - #510

Merged
Gui-FernandesBR merged 4 commits into
developfrom
bug/fix-fin-flutter
Dec 20, 2023
Merged

BUG: fin_flutter_analysis doesn't find any fin set#510
Gui-FernandesBR merged 4 commits into
developfrom
bug/fix-fin-flutter

Conversation

@Gui-FernandesBR

@Gui-FernandesBR Gui-FernandesBR commented Dec 14, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Pull request type

  • Code changes (bugfix, features)
  • Code maintenance (refactoring, formatting, tests)

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest --runslow) have passed locally
  • CHANGELOG.md has been updated (if relevant)

Current behavior

This bug was reported through discord yesterday:
image

Also, the tests for the fin_flutter_analysis were deactivated since they were not passing.

New behavior

  • Solved the bug
  • Refactored the code to improve readability.
  • Improved the tests for flutter analysis.

Breaking change

  • No

Additional information

This feature works only for trapezoidal fins, that's said.
Also, we should improve the documentation in the future, try to give some more examples.

@Gui-FernandesBR
Gui-FernandesBR requested a review from a team as a code owner December 14, 2023 07:46
@Gui-FernandesBR

Gui-FernandesBR commented Dec 14, 2023

Copy link
Copy Markdown
Member Author

I am still working on unit tests to prevent from future occasions. Please don't review it yet

@codecov

codecov Bot commented Dec 14, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (f1367e3) 71.30% compared to head (5390478) 72.02%.
Report is 2 commits behind head on develop.

❗ Current head 5390478 differs from pull request most recent head 6f4c3c3. Consider uploading reports for the commit 6f4c3c3 to get more accurate results

Files Patch % Lines
rocketpy/utilities.py 88.23% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #510      +/-   ##
===========================================
+ Coverage    71.30%   72.02%   +0.72%     
===========================================
  Files           56       56              
  Lines         9374     9370       -4     
===========================================
+ Hits          6684     6749      +65     
+ Misses        2690     2621      -69
Flag Coverage Δ
unittests 72.02% <88.23%> (+0.72%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.X.0 milestone Dec 14, 2023
@Gui-FernandesBR Gui-FernandesBR added Bug Something isn't working Tests Regarding Tests Refactor labels Dec 14, 2023
phmbressan
phmbressan reviewed Dec 15, 2023
View reviewed changes
Comment thread rocketpy/utilities.py Resolved
Gui-FernandesBR
Gui-FernandesBR commented Dec 15, 2023
View reviewed changes
Comment thread rocketpy/utilities.py
found_fin = False

# First, we need identify if there is at least a fin set in the rocket
for aero_surface in flight.rocket.aerodynamic_surfaces:

@Gui-FernandesBR Gui-FernandesBR Dec 15, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just to make reviewer's life easier: this was the problematic code line. The aerodynamic_surfaces is no longer a list of surfaces, instead it is a list of components;

phmbressan
phmbressan approved these changes Dec 19, 2023
View reviewed changes

@phmbressan phmbressan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good to see these tests reintroduced, let's hope they don't give the same headaches as before.

Comment thread rocketpy/utilities.py Resolved