-
Notifications
You must be signed in to change notification settings - Fork 54
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
Starting Backintegration of Tutorial Notebooks #334
Starting Backintegration of Tutorial Notebooks #334
Conversation
- the 1st Basic Notebook Idealized-Case-1_Tracking-of-a-Test-Blob-in-2D.ipynb has been adjusted such that it runs with v1.5.x - testdata are already provided as xarray (!) - minimum of decorators has been added to tobac functions to allow the necessary xarray input to feature detection, etc.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## RC_v1.5.x #334 +/- ##
=============================================
+ Coverage 56.88% 57.00% +0.12%
=============================================
Files 20 20
Lines 3444 3454 +10
=============================================
+ Hits 1959 1969 +10
Misses 1485 1485
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…tle -> this is too obvious
Hi all, I am coming closer to finish backintegration of tutorials @JuliaKukulies : I got your cyclone tracking example in again :-D The two notebooks I have not touched so far are
In the first, GOES data had been retrieved from AWS using boto3. This appears to be broken ... @w-k-jones : You are using s3fs in the 2nd notebook to do the same. Could confirm that this is the smarter way to go? In the second, I would rather use proj for projection calculations and would also re-structure it a bit. @w-k-jones : Would you be okay with this? @freemansw1 : This provides already a few templates how to work with the decorators (Basics + Cyclone Example). I hope this is not in conflict with the typehint development. Cheers |
I think @freemansw1 did most of the work on these notebooks, but using |
Linting results by Pylint:Your code has been rated at 8.72/10 (previous run: 8.72/10, +0.00) |
Two discoveries from today:
The "Example_VIS_Tracking_Satellite.ipynb" notebook uses a lot of memory (>14GB), we need to look into reducing this to make it better able to run on personal computers and CI |
Yes, apologies the notebook was broken due to a previous merge conflict so I reverted to the previous version. Let me see if I can grab the updated version |
Are there any other updates to this notebook other than our restructuring? |
@w-k-jones : No worries! I saw that I need to update some descriptive parts of this notebook anyway ... I will do it in the next hour |
Just realized that the example is using GOES Band 10 7.3 um -> lower-level water vapor. I guess this is a mistake and window channel was intended? |
@w-k-jones: Following changes:
|
@w-k-jones & @fziegner : I selected you as reviewers for this PR. Should be not too much work ... All checks passed and several pairs of eyes have been already looking at the content. @fziegner : Could you please carefully check if some minor correction you maybe implemented, did not disappear during merge with RC1.5.x? Thanks! |
The only difference I can see is the loading of files with xarray rather than iris, but I think we should stick to iris until v1.6 |
Quick question on these... maybe this is another PR, but do we want these visible (in some way, shape, or form) from our documentation page? |
Thanks, Sean. I added the issue #409 to keep us reminded. Cheers, Fabian. |
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.
Happy for this to be merged
@w-k-jones : Thanks! |
Can I move the milestone here to 1.5.3? |
I think that is a good idea |
Good suggestion! I would love push this PR (as one of our tobathon targets) |
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.
@fsenf I opened a PR on your fork for adding a save function to the Radar/Satellite notebook. Otherwise everything looks good to me.
Added save function to Track_on_Radar_Segment_on_Satellite notebook
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.
Great, should be good to go now.
@w-k-jones & @fziegner : Thank you for your additions! I will now carry out the merge. |
This starts backintegration of tutorial notebooks and is far from being complete. It aims to solve #282
This will be updated in the upcoming days/weeks (you know ...). Please treat it for now as a draft until all is complete.
Check list for tutorials:
Check list for tobac examples: