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

Getting Started Notebook Fails #114

Closed
giovaniceotto opened this issue Dec 8, 2021 · 7 comments · Fixed by #106
Closed

Getting Started Notebook Fails #114

giovaniceotto opened this issue Dec 8, 2021 · 7 comments · Fixed by #106
Assignees
Labels
Docs Docs and examples related

Comments

@giovaniceotto
Copy link
Member

giovaniceotto commented Dec 8, 2021

Describe the bug

When running the getting started notebook (which can be done via Colab), it fails and throws and error in the Dynamic Stability Analysis section.

The error happens because a typo was fixed in every file of the master branch, including in the notebook (#106). The typo in question was the incorrect spelling "Costum" instead of "Custom", which affects the "CustomAtmosphere" argument that can be used in the Environment.setAtmosphericModel method. However, the current version of rocketpy released on PYPI (0.9.8) does not yet incorporate this typo fix.

To Reproduce

  1. Access the Getting Started notebook via Google Colab
  2. Run the entire notebook (all the cells, in order)
  3. Notice ValueError: Unknown model type after running the Dynamic Stability Analysis cell

Expected behavior

The entire notebook should run without errors.

Screenshots

image

Solution Sugestion

Release a new version!

@giovaniceotto giovaniceotto linked a pull request Dec 8, 2021 that will close this issue
9 tasks
@Gui-FernandesBR
Copy link
Member

Tks for sharing this!

Version Release will be adressed as soon as possible, I'll handle it myself.

@Gui-FernandesBR
Copy link
Member

Hey, sorry for the failure on the asap part.
Still, I'd like to handle it myself anyway, I need to learn how to use the automatic release on PyPI package.
I'm creating here a deadline of 5days, so until 09th january this may be released.

@giovaniceotto can you help me to understand if we need to merge anything from "develop" do "master" before realeasing the new version? If positive, let's select the pull request and create the merge.

@giovaniceotto
Copy link
Member Author

@Gui-FernandesBR, I do believe we may have a couple of things in the develop branch that could be merged into master before the new version. Could we discuss it further over discord, together with the automatic release?

@Gui-FernandesBR
Copy link
Member

Great idea Gio! I will propose an appointment on your agenda for tomorrow and then we meet on discord

@Gui-FernandesBR
Copy link
Member

Last release was created on 16th August. Since that day, the following PR were merged on develop branch:

@MrGribel could you please check and validate the proposal giving your OK (or NOK) for each line?
We need this before proceeding with the process of merging develop into master

@MrGribel
Copy link
Contributor

MrGribel commented Jan 7, 2022

Master branch should contain stable code. I doubt we have many users extensively testing our dev branch since it is not the default one, meaning we ourselves need to check if it is ready to deploy. Everything seems to be running well on my end and imo these PRs are too unrelated to have broken anything hidden due to conflict.

Just a thing: develop currently has a CI build error that should probably be adressed first. After that, feel free to merge and create a new release.

EDIT: Just noticed that since this is our first time merging from dev: it is 74 commits behind master...
It might require some care if any merge conflicts arise. Perhaps a temp staging branch could be a good idea to test everything beforehand.

@Gui-FernandesBR
Copy link
Member

Works fine now, tks to all team!

@Gui-FernandesBR Gui-FernandesBR added the Docs Docs and examples related label Sep 21, 2022
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
None yet
Development

Successfully merging a pull request may close this issue.

3 participants