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

DOC: Installing imageio library on dispersion analysis notebook #540

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

Lucas-Prates
Copy link
Contributor

@Lucas-Prates Lucas-Prates commented Jan 25, 2024

Pull request type

  • ReadMe, Docs and GitHub updates

Checklist

  • Docs have been reviewed and added / updated

Current behavior

The dispersion analysis notebook uses, on its last cell, the library imageio, but does not install it at any point, nor is it a dependency. Executing the whole notebook without having the library prompts a "ModuleNotFoundError" at the last cell.

New behavior

Install imageio at the 'Install and Load Libraries' section.

Breaking change

  • No

Additional information

The installation is done exclusively for this notebook, so there are no changes to dependencies.

@Lucas-Prates Lucas-Prates changed the base branch from master to develop January 25, 2024 17:27
@Lucas-Prates Lucas-Prates requested a review from a team as a code owner January 25, 2024 17:27
@Lucas-Prates Lucas-Prates added Docs Docs and examples related Monte Carlo Monte Carlo and related contents and removed Monte Carlo Monte Carlo and related contents labels Jan 25, 2024
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (821518a) 72.34% compared to head (b469931) 72.34%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #540   +/-   ##
========================================
  Coverage    72.34%   72.34%           
========================================
  Files           56       56           
  Lines         9393     9393           
========================================
  Hits          6795     6795           
  Misses        2598     2598           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.X.0 milestone Jan 25, 2024
@Gui-FernandesBR
Copy link
Member

In order to test it on colab: https://colab.research.google.com/github/RocketPy-Team/rocketpy/blob/master/docs/notebooks/dispersion_analysis/dispersion_analysis.ipynb

This link clones from master branch, so we need to modify the pip install command before running all the cells.

@Gui-FernandesBR
Copy link
Member

The code is running as expected here.

In the future we might wanna change the implementation to fix this warning:

<ipython-input-26-957e0ff3cd5e>:6: DeprecationWarning: Starting with ImageIO v3 the behavior of this function will switch to that of iio.v3.imread. To keep the current behavior (and make this warning disappear) use `import imageio.v2 as imageio` or call `imageio.v2.imread` directly.
  img = imread("dispersion_analysis_inputs/Valetudo_basemap_final.jpg")

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Works on colab lol

Thanks a lot and congratulations for your first contribution, @Lucas-Prates !!!

Let's wait until our meeting so we let other people see the PR before merging :)

@phmbressan phmbressan merged commit 2fee77e into develop Jan 25, 2024
@phmbressan phmbressan deleted the doc/dispersion_analysis_imageio branch January 25, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Docs and examples related
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants