Skip to content

REL: Welcome to v0.13.0 #323

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 189 commits into from
Jan 22, 2023
Merged

REL: Welcome to v0.13.0 #323

merged 189 commits into from
Jan 22, 2023

Conversation

Gui-FernandesBR
Copy link
Member

Pull request type

Please check the type of change your PR introduces:

  • Other (please describe): Release!

Pull request checklist

This is a release PR, all changes were already approved previously ;)
Tests are passing, of course, and docs were updated.

What is the current behavior?

The current master branch is outdated. A lot of changes have been made to develop branch and, finally, I believe that we are able to merge everything to master, upgrading our python package version.

What is the new behavior?

Several changes, really! I will try to summarize them below:

  • Environment Analysis 2.0: improved, tested, and examplified.
  • Fins sweep angle implemented
  • Integration with Windy/ECMWF
  • New AeroSurfaces class
  • postProcess method is died, say hi to cached properties!
  • Timezonefinder is now optional

Does this introduce a breaking change?

  • Yes
  • No

Other information

Please wait for merge conflicts to be solved before reviewing!

@Gui-FernandesBR Gui-FernandesBR added the Enhancement New feature or request, including adjustments in current codes label Jan 10, 2023
@Gui-FernandesBR Gui-FernandesBR added the Releases Related with new releases label Jan 10, 2023
@Gui-FernandesBR
Copy link
Member Author

Conflicts were solved and tests are running,

@giovaniceotto and @MateusStano could you guys review this one pls?
After merging, we can create a new release so the PyPI package can be updated.

I'll wait for review of both of you before merging, this is a large PR.

@giovaniceotto
Copy link
Member

@Gui-FernandesBR, here is my first review. As this is quite a large PR, I decided to review half of the files today and will review the other half tomorrow, although I have already looked through all the files. Let me know if you have any questions about my comments.

@giovaniceotto
Copy link
Member

I'll need one more day to run a couple of checks before we are ready to go. Almost there!

@giovaniceotto
Copy link
Member

Here is a problem with the documentation, which can be checked here: https://docs.rocketpy.org/en/develop/user/index.html

It seems the Environment Analysis notebook is using H1 headers instead of H2 for Surface Level Analysis, Pressure Level Analysis and Going Further with your Analysis, which causes the main index of the user guide page to get cluttered.

Furthermore, the utilities model notebook is out of date regarding the new function create_dispersion_dictionary.

When these issues (and the docstring for create_dispersion_dictionary function) are fixed, I will accept the changes and this PR will be able to be merged.

@Gui-FernandesBR
Copy link
Member Author

Ok, now tests are passing, and the documentation seems to be good. I also added two notebooks to index.rst file, what I cannot is saying why they weren't included before.

Furthermore, the utilities model notebook is out of date regarding the new function create_dispersion_dictionary

Hey, I don't think it should be mandatory documenting all the new utilities functions on notebooks. Other classes don't even have a notebook.... Therefore, I vote for skipping this one. Maybe in the future we can provide that. The current utilities module notebook seems good to me.

Everything else was addressed, at least temporarily.

Btw sorry for long commit list, it turns out some tests were passing locally without running so I committed some poor lines thinking that it was running well.

@giovaniceotto
Copy link
Member

@Gui-FernandesBR, I have fixed the documentation problem related to headings 1 being used mistakenly. I have also defined which exception to catch in the create_dispersion_dictionary function and replaced prints with warnings. Please review this changes.

Once done, I believe we are ready to merge. Quick idea: wait until the weekend (just one day) so that we can host a launch party in our discord channel with the new members so that they can see how a release is done (and so that we can promote this new version together)!

@Gui-FernandesBR
Copy link
Member Author

I got an idea: use quotes to separate objects in the csv file!!

@MateusStano
Copy link
Member

We are releasing this today!

@MateusStano MateusStano merged commit 61a21d2 into master Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request, including adjustments in current codes Releases Related with new releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants