Skip to content

Comments

Update tutorial notebooks#73

Merged
tennlee merged 4 commits intoACCESS-Community-Hub:developfrom
JMP-MO:update-tutorial-notebooks
Mar 28, 2025
Merged

Update tutorial notebooks#73
tennlee merged 4 commits intoACCESS-Community-Hub:developfrom
JMP-MO:update-tutorial-notebooks

Conversation

@JMP-MO
Copy link
Collaborator

@JMP-MO JMP-MO commented Mar 27, 2025

Some of the tutorial notebooks were not working due to method/class name changes in the packages which haven't been updated in the tutorial notebooks. An example of this is standard_longitude which is now StandardLongitude.

I have also made sure there is a clear explanation of how to access the local ERA5lowres data at NCI, NIWA and the Met Office with links to all 3 organisation locations in each notebook.

I have also added some additional explanation in the notebooks to try and help new users. For instance I have added context explaining how the CNN ML project works and what it is trying to predict which is then referred back to later when explaining how input / target data indexing works.

There is also an update to the IPython import in the pipeline.controller file as this import was failing with .core

@nikeethr
Copy link
Collaborator

@JMP-MO thanks for this. I hit a similar issue. As such I might use (and implicitly review) this PR and see if it fixes what I'm experiencing.

@nikeethr
Copy link
Collaborator

@JMP-MO thanks for the added descriptions its very helpful, and of course fixing the standard longitude renaming. I have some minor comments above.

I have yet to run the notebooks, but will do so soon and see if there's any regression from my end.

@coveralls
Copy link

coveralls commented Mar 28, 2025

Pull Request Test Coverage Report for Build 14123954011

Details

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 55.032%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/pipeline/src/pyearthtools/pipeline/controller.py 0 1 0.0%
Totals Coverage Status
Change from base Build 14099993677: 0.0%
Covered Lines: 7331
Relevant Lines: 12866

💛 - Coveralls

@tennlee
Copy link
Collaborator

tennlee commented Mar 28, 2025

This all looks great to me. I added a minor bugfix which I suspect may have just been accidentally not included in the commit for some reason. I will shortly merge it, and have moved the comments into a new issue to tackle without holding up the PR.

@tennlee
Copy link
Collaborator

tennlee commented Mar 28, 2025

I will merge this momentarily despite the CI failures on the pre-commit hook. I reran the black formatter to try to resolve this, and it's unclear to me why that didn't produce a compliant file. I will investigate shortly and push up a fix to the formatting separately.

@tennlee tennlee merged commit e113a4f into ACCESS-Community-Hub:develop Mar 28, 2025
5 of 6 checks passed
@JMP-MO
Copy link
Collaborator Author

JMP-MO commented Mar 28, 2025

Thanks Tennessee. I hope the additional explanations are useful, being brand new to the project provides an opportunity to experience what it is like getting started with the tool and I tried to explain the initial roadblocks or questions I had as I worked through the material. I might take a look at the CNN scaling code with Joel next week as I haven't seen scaling approached like that. I am guessing its to do with saving out the statistics to make the processes reversible later, whereas usually I would save the scaler object. I was saying to Stephen that developing some PET ML Pipeline Templates could be really useful for users to jump straight into ML.

@tennlee
Copy link
Collaborator

tennlee commented Mar 28, 2025

Fixes #71 and #70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Naming of methods/classes have changed in packages affecting tutorial notebooks cannot import display from IPython.core.display

4 participants