Skip to content

Conversation

@phmbressan
Copy link
Collaborator

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:

  • 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?

The default rocketpy install through pip does not install the optional dependencies as intended. However, importing the package loaded the EnvironmentAnalysis module that required them, raising an error.

What is the new behavior?

The imports of the optional modules are made lazily, i.e. on instantiation of an EnvironmentAnalysis object. Then, the expected install behavior is assured.

Does this introduce a breaking change?

  • Yes
  • No

Other information

The way the imports were done goes against PEP8 convention of only handling imports in top level statements. However, the implemented way is simpler for just these few dependencies. In the long term, if more complex install conditions are needed, an file to process this requirements (as in pandas repository) may be considered.

@phmbressan phmbressan added the Bug Something isn't working label Aug 17, 2023
@phmbressan phmbressan self-assigned this Aug 17, 2023
Copy link
Member

@MateusStano MateusStano left a comment

Choose a reason for hiding this comment

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

I believe we are still missing changes on the imports of environment_analysis_plots.py and environment_analysis_prints.py for this PR to work

@phmbressan phmbressan closed this Aug 19, 2023
@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.0.0 milestone Sep 6, 2023
@Gui-FernandesBR Gui-FernandesBR deleted the fix/optional-imports branch September 6, 2023 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants