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

Generate Histograms for 2D field #594

Merged
merged 82 commits into from
Jul 23, 2024
Merged

Conversation

Sylviabohnenstengel
Copy link
Contributor

@Sylviabohnenstengel Sylviabohnenstengel commented May 9, 2024

Description

generating a PDF for a 2D field looping over vertical levels

Fixes #453

Contribution checklist

Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.

  • Documentation has been updated to reflect change.
  • New code has tests, and affected old tests have been updated.
  • All tests and CI checks pass.
  • Added an entry to the top of docs/source/changelog.rst
  • Conda lock files have been updated if dependencies changed.
  • Marked the PR as ready to review.
  • has ensemble capability
  • AI was used

@Sylviabohnenstengel Sylviabohnenstengel self-assigned this May 9, 2024
@jfrost-mo jfrost-mo changed the title 543 generate pdfs for 2d field Generate PDFs for 2D field May 10, 2024
@Sylviabohnenstengel
Copy link
Contributor Author

adding ensemble capability

@Sylviabohnenstengel Sylviabohnenstengel added science Scientific capabilities full_review Requires a technical, scientific, and portability review labels May 15, 2024
@jfrost-mo jfrost-mo changed the title Generate PDFs for 2D field Generate Histograms for 2D field May 23, 2024
@jfrost-mo jfrost-mo changed the base branch from main to vertical_profiles_of_area_averaged_fields_494 May 24, 2024 13:16
Base automatically changed from vertical_profiles_of_area_averaged_fields_494 to main May 29, 2024 09:07
@jfrost-mo
Copy link
Member

Old branch head: 2d9ead7

@jfrost-mo jfrost-mo force-pushed the 543_generate_pdfs_for_2d_field branch from 2d9ead7 to ba36195 Compare June 25, 2024 10:16
Copy link
Contributor

github-actions bot commented Jun 25, 2024

Coverage

@jfrost-mo
Copy link
Member

Note to self: There are a bunch of conflict markers in this branch's code that need cleaning up.

@Sylviabohnenstengel
Copy link
Contributor Author

rebased the branch

@Sylviabohnenstengel
Copy link
Contributor Author

code working and now need to add unit test.
image

@Sylviabohnenstengel
Copy link
Contributor Author

GitHub Copilot used to debugfilename in assert Path("test_473718.0.png").is_file()

@Sylviabohnenstengel Sylviabohnenstengel marked this pull request as ready for review July 3, 2024 17:14
@Sylviabohnenstengel
Copy link
Contributor Author

@daflack @jfrost-mo I suggest that James does the technical review first and once that has passed David can do the science review.

…inking new recipe into workflow, including rose meta information and rose config infromation. Adding recipe and flow files.
New recipe to calculate spatial mean on vertical levels.
…le plotting operator to plot vertical profile.
…hanges from main. include vertical line plot operator and internal function.
…gh recipe as it splits on whitespace even inside "1000, 850". even when passing in the values as [1000,850, then jinja2 introduces a white space converting to [1000, 850] and then the split function splits the list up.
Copy link
Member

@jfrost-mo jfrost-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plotting code changes look good. One of the include files has an unnecessary loop, and more tests need writing for the new plotting functionality.

cset-workflow/meta/rose-meta.conf Outdated Show resolved Hide resolved
cset-workflow/meta/rose-meta.conf Outdated Show resolved Hide resolved
cset-workflow/meta/rose-meta.conf Outdated Show resolved Hide resolved
cset-workflow/meta/rose-meta.conf Outdated Show resolved Hide resolved
cset-workflow/meta/rose-meta.conf Outdated Show resolved Hide resolved
src/CSET/operators/plot.py Outdated Show resolved Hide resolved
src/CSET/operators/plot.py Outdated Show resolved Hide resolved
src/CSET/operators/plot.py Outdated Show resolved Hide resolved
tests/operators/test_plots.py Show resolved Hide resolved
Sylviabohnenstengel and others added 9 commits July 19, 2024 13:05
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
@Sylviabohnenstengel
Copy link
Contributor Author

Plotting code changes look good. One of the include files has an unnecessary loop, and more tests need writing for the new plotting functionality.

It has the wrong statement in the loop. IT should read:
{% for model_field in PRESSURE_LEVEL_MODEL_FIELDS %}

Corrected now in 11fe6d7..8344e50

@Sylviabohnenstengel
Copy link
Contributor Author

Plotting code changes look good. One of the include files has an unnecessary loop, and more tests need writing for the new plotting functionality.

This has been changed in the code and this version is outdated.

@jfrost-mo
Copy link
Member

Test coverage look reasonable. There are still a few lines added that are uncovered, but it'll do for now.

Copy link
Member

@jfrost-mo jfrost-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Go ahead and merge.

@jfrost-mo jfrost-mo dismissed daflack’s stale review July 22, 2024 16:36

David approved in another comment, but forgot to dismiss the existing review.

@jfrost-mo jfrost-mo merged commit cce0d35 into main Jul 23, 2024
7 checks passed
@jfrost-mo jfrost-mo deleted the 543_generate_pdfs_for_2d_field branch July 23, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full_review Requires a technical, scientific, and portability review science Scientific capabilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generate histograms for 2D fields on vertical levels
3 participants