Skip to content

MAINT: move dispersion plots #375

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

Merged
merged 5 commits into from
Oct 29, 2023

Conversation

Gui-FernandesBR
Copy link
Member

Pull request type

Please check the type of change your PR introduces:

  • Code maintenance (refactoring, formatting, renaming, tests)

Pull request checklist

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

  • 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

What is the current behavior?

Some plots and prints still inside the Dispersion Class.

What is the new behavior?

New dedicated classes to _DispersionPlots and _DispersionPrints

Does this introduce a breaking change?

  • Yes (functions being moved from one module to another (submodules)

Other information

  • Let's merge this one into the enh/class_dispersion :)
  • Updating the notebooks changes more than 8000 rows, when actually the PR changed at maximum 400 rows.

@Gui-FernandesBR Gui-FernandesBR added Refactor Monte Carlo Monte Carlo and related contents labels Jun 11, 2023
@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.0.0 milestone Jun 11, 2023
@Gui-FernandesBR Gui-FernandesBR self-assigned this Jun 11, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Gui-FernandesBR
Copy link
Member Author

Tests not passing are not related to this PR. Basically, the Dispersion class is currently not working not even in the enh/class_dispersion branch.
But I solved what the PR asked, so the re-review was requested.

@Gui-FernandesBR
Copy link
Member Author

The review was accomplished, merging this one without approval to fix the problems directly on the enh/class_dispersion branch

@Gui-FernandesBR Gui-FernandesBR merged commit 59b0d94 into enh/class_dispersion Oct 29, 2023
@Gui-FernandesBR Gui-FernandesBR deleted the maint/move-dispersion-plots branch November 9, 2023 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monte Carlo Monte Carlo and related contents Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants