-
Notifications
You must be signed in to change notification settings - Fork 124
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
Update IBTrACS to version 4.1 #976
Conversation
Thanks a lot for this PR, @ChrisFairless! 🙌 I am currently on parental leave and will not be able to review until January. Maybe somebody else would be willing? @spjuhel @NicolasColombi |
Happy to give this a look next week! |
Happy to have a look as well! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks in order for me.
👍 for replacing the hard-coded filenames!
Thank you @ChrisFairless! All good from my side. |
Please do NOT merge before solving the problem in the tutorial. This is the entire point of this PR. Also, we really do not want to push things that we know are failing, this is the perfect recipe for accumulating errors that will never be addressed. Not addressing an error is only acceptable if we know the reason for the error. |
Yeah it looks like https://www.ncei.noaa.gov/data/ is down today. I get a 503 'Service unavailable' regardless of what I'm accessing. They had some issues earlier in the year too. We can wait and re-run the builds once it's back online. |
Thanks for checking! Let's wait for NOAA to be available again, and if it then runs let's merge. |
Ok, I fixed some minor issues with TCTracks and Hazard on the go. The unit tests are currently failing, not so clear why. Part of it is due to changes in petals, part is issues with the pre-commit hook that is failing with code 1, and part is internet connections. Let's try again later and see which ones are temporary. |
Compatibility test with petals failing because the tests used the 4.0 version. @ChrisFairless please check CLIMADA-project/climada_petals#152 |
@chahank @ChrisFairless @NicolasColombi : time to merge - right? |
I just updated the changelog to make it clearer. For me it is good to go. |
🙌 to all )I manually re-ran the failing compatibility tests on jenkins: apparently just a temporary glitch.) |
Changes proposed in this PR:
This year IBTrACS updated from version 4.0 to 4.1 and on May 28th the last v4.0 dataset was posted to their site.
That means anyone using CLIMADA is using IBTrACS data that hasn't been updated since then, even if they're regularly redownloading the file.
This PR points everything to the location of the new download and updates the tests because some of the tested values have changed.
Changes in v4.1:
The (rewritten) tests for the unit all pass.
Note:
PR Author Checklist
develop
)PR Reviewer Checklist