ENH: Refactor Rocket.addFins to avoid breaking changes

[giovaniceotto]

Pull request type

Please check the type of change your PR introduces:

• [ ] Code base additions (bugfix, features)
• [x] Code maintenance (refactoring, formatting, renaming, tests)
• [ ] ReadMe, Docs and GitHub maintenance
• [ ] Other (please describe):

Pull request checklist

Please check if your PR fulfills the following requirements, depending on the type of PR:

• Code base maintenance (refactoring, formatting, renaming):

  • [x] Docs have been reviewed and added / updated if needed
  • [x] Lint (black rocketpy) has passed locally and any fixes were made
  • [ ] All tests (pytest --runslow) have passed locally

What is the current behavior?

The proposed implementation of Rocket.addFins (#172 ) will cause breaking changes.

What is the new behavior?

The new proposed implementation creates two methods: Rocket.addTrapezoidalFins and Rocket.addEllipticalFins to avoid breaking changes. Furthermore, it marks Rocket.addFins as a future deprecation to be removed in version 2.0.0 (to be discussed).

Does this introduce a breaking change?

• [ ] Yes
• [x] No

Other information

Work to fix notebooks and docs that mention addFins must be worked on to reflect theses changes.

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[MateusStano]

Hey! This seems to fix the breaking change, I just fixed a few small bugs regarding the radius parameter

Just wondering about some repeated code on both new functions, like the functions def beta(mach) and def finNumCorrection(n) or the cl calculations. Wouldn't it be better split up de addFins functions into even more functions? Or create a class maybe?

[giovaniceotto]

@MateusStano, I completely agree that it is about time to create a class to manage fins and perhaps even the remaining aerodynamic surface.

How about something like this in the future?

from rocketpy import Rocket
from rocketpy.aerodynamics import BoatTail, TrapezoidalFins, ElipticalFins, NoseCone

TestRocket = Rocket(...)
TestFins = TrapezoidalFins(...)

TestRocket.addAerodynamicSurface(TestFins)

With something like this, both TrapezoidalFins and ElipticalFins could be a class inherited from the same Fins class, which could contain the repeated code. Furthermore, the base Fins class could isnheret from a even more basic and abstract AerodynamicSurface class.

What do you think?

The only issue is that this causes breaking changes. So I would advise releasing the ElipticalFins implementation first and then create theses changes.

[MateusStano]

I really like the approach for the classes. I think it would make the code simpler to understand and easier to expand. Maybe instead of using inheriting abstract calss would fit better for teh Aero Surfaces class for example

I guess the best way then is to merge this PR and leave the Aerodynamic Surface and Fin class to some other time