ENH: Implement optional plot saving

nalquas commented 1 month ago

Pull request type

[x] Code changes (bugfix, features)

Checklist

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

Current behavior

Currently, the draw functions responsible for most plots use matplotlib's show function. There is currently no (supported) way to automatically save the plots to disk. See #564

New behavior

The functions generating plots now have a filename parameter, by default None. If the filename is None, the plot is shown as before using matplotlib's show. If the filename is a file path string (e.g. "/absolute/path/to/file.png" or "relative/path/to/file.jpg"), the plot is saved to the specified file. The file ending in the given filename decides which format the plot will be saved in, with matplotlib supporting: eps, jpg, jpeg, pdf, pgf, png, ps, raw, rgba, svg, svgz, tif, tiff and webp.

Breaking change

[x] No (only added optional parameters)

Additional information

I did not modify functions calling several plot-drawing functions, e.g. functions such as all(...). The reason for this is that those are probably only used for manual testing/analysis, e.g. when using Jupyter Notebook. Users wanting automatic output probably prefer calling the individual plot functions directly as it allows for more fine-control. Additionally, this saves me from additional implementation effort 😅, and reduces complexity of this pull request. If this functionality is still wanted, this could be implemented in a future pull request.

Also, I may have missed some plotting functions. If you spot anything obvious missing, feel free to notify me and I will look into it.

I look forward to any feedback during code review.

Fixes #564

Gui-FernandesBR commented 1 month ago

Hey, thank you so much for your draft PR @nalquas , it looks good already.

I will take a look at it by the end of this week and see if I find any improvement opportunity.

Nice job.

nalquas commented 1 month ago

The PR is now ready for review. I did not modify the tests, though, and I am not sure about what needs to be done to get the docs updated.

Looking forward to your feedback!

Gui-FernandesBR commented 1 month ago

I also would like to run code coverage on this PR, the workflow did not worked because the base branch is coming from a fork. I need to investigate it better.

Gui-FernandesBR commented 1 month ago

Btw @nalquas could you please run isort to sort the imports in the files you modified?

You could use this command: isort --profile black rocketpy/ tests/ docs/

You can run pip install isort if needed

nalquas commented 1 month ago

I have applied the suggestions, updated the docstrings, moved get_matplotlib_supported_file_endings into tools.py and run isort/black. I have also rebased the PR onto the current develop branch.

Feel free to review again 🙂

RocketPy-Team / RocketPy