Skip to content
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/parachute #108

Merged
merged 14 commits into from Nov 21, 2021
Merged

Enh/parachute #108

merged 14 commits into from Nov 21, 2021

Conversation

FranzYuri
Copy link
Contributor

Pull request type

Please check the type of change your PR introduces:

  • Code base additions (bugfix, features)
  • 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:

  • ReadMe, Docs and GitHub maintenance:

    • Spelling has been verified
    • Code docs are working correctly
  • Code base maintenance (refactoring, formatting, renaming):

    • Docs have been reviewed and added / updated if needed
    • Lint (black rocketpy) has passed locally and any fixes were made
    • All tests (pytest --runslow) have passed locally
  • Code base additions (for bug fixes / features):

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

What is the current behavior?

Currently there is no place to keep functions that are useful but do not belong to any of the 5 core classes in Rocketpy.
One of these functions is the compute_cds_from_drop_test, that calculates the CdS of a parachute from a drop test.

What is the new behavior?

With the module utilities.py, it is possible to save functions like the compute_cds_from_drop_test.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Enter text here...

ENH: add utilities.py module to rocketpy.

There are several functions that are useful to RocketPy but should not belong to any class in the software.  Therefore, a module that keeps those functions may be benefitial to RocketPy.
@Lucas-KB
Copy link
Contributor

Lucas-KB commented Nov 4, 2021

@FranzYuri, I would suggest you run black to reformat your code:

  1. Install black running pip install black
  2. Run black rocketpy when in the repository folder

The second thing is that it may be necessary to add something like from .utilities import * or from .utilities import compute_cds_from_drop_test to the __init__.py file, so the import process can be achieved using from rocketpy import *.

@Gui-FernandesBR
Copy link
Member

Currently there is no place to keep functions that are useful but do not belong to any of the 5 core classes in Rocketpy. One of these functions is the compute_cds_from_drop_test, that calculates the CdS of a parachute from a drop test.

Can you guys please indicate other functions with that same behavior in the code?

@Lucas-KB
Copy link
Contributor

Lucas-KB commented Nov 9, 2021

@Gui-FernandesBR for now I haven't personally identified any function that applies. But I imagine we could use this file in the future to gather functions to help design Rockets/Parachutes/Motors etc. For example functions related to rocket parameter optimizations (ideal mass for a certain motor or something similar). I think the suggestion to create utilities.py came from @giovaniceotto and @FranzYuri, maybe they had different ideas.

@Gui-FernandesBR
Copy link
Member

Tks for explaning that, @Lucas-KB . If I'm understamdimg this correctly, I have some candidates to be included at utilities.py, let's see:

Environment:
def decimalDegressToArcSeconds(self, angle):

Flight:
def calculateFinFlutterAnalysis(self, finThickness, shearModulus):
def exportPressures(self, fileName, timeStep):

Those could help to give more "body" to this new .py file, and they could be moved in another PR I guess. Please let me know if these suggestions are good or not.

Also, I would recommend creating a simple example usage for these functions. At least for me, it is not easy to visualize how I could use the new capabilities.

@giovaniceotto
Copy link
Member

Excellent suggestions @Gui-FernandesBR! I hope you are ok with me creating a new issue regarding your ideas so that we can come back to them in a future PR. The issue is here #111.

Copy link
Member

@giovaniceotto giovaniceotto left a comment

Choose a reason for hiding this comment

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

Looks awesome! Just a few details to iron out and then we can merge this PR, which marks a new path for RocketPy functionalities.

rocketpy/utilities.py Outdated Show resolved Hide resolved
rocketpy/utilities.py Outdated Show resolved Hide resolved
rocketpy/utilities.py Outdated Show resolved Hide resolved
rocketpy/utilities.py Outdated Show resolved Hide resolved
@Gui-FernandesBR
Copy link
Member

Don't need my permission @giovaniceotto ... Thanks for opening the 111, I appreciate it

FranzYuri and others added 5 commits November 11, 2021 16:35
Co-authored-by: Giovani Hidalgo Ceotto <ghceotto@gmail.com>
Co-authored-by: Giovani Hidalgo Ceotto <ghceotto@gmail.com>
Co-authored-by: Giovani Hidalgo Ceotto <ghceotto@gmail.com>
Co-authored-by: Giovani Hidalgo Ceotto <ghceotto@gmail.com>
Adding a module utilities to keep usefull functions to RocketPy.
@FranzYuri
Copy link
Contributor Author

@giovaniceotto I think it is alright now, can you take a look?

@giovaniceotto
Copy link
Member

@FranzYuri, my bad, I had reviewed your changes but forgot to approve it!

Looks perfect now. Would you do the honors of merging the pull request?

@FranzYuri FranzYuri merged commit 392c290 into develop Nov 21, 2021
@Gui-FernandesBR Gui-FernandesBR deleted the enh/parachute branch May 29, 2022 14:39
@Gui-FernandesBR Gui-FernandesBR added Enhancement New feature or request, including adjustments in current codes NewFeature labels Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request, including adjustments in current codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants