Skip to content

Conversation

@daflack
Copy link
Contributor

@daflack daflack commented Dec 19, 2024

Adds Contour Frequency by Altitude (CFAD) into CSET.

Fixes #1010

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.
  • Ensured the pull request title is descriptive.
  • Conda lock files have been updated if dependencies have changed.
  • Attributed any Generative AI, such as GitHub Copilot, used in this PR.
  • Marked the PR as ready to review.

@daflack daflack added the enhancement New feature or request label Dec 19, 2024
@daflack daflack self-assigned this Dec 19, 2024
@daflack daflack marked this pull request as draft December 19, 2024 16:14
@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2024

Coverage

@daflack daflack force-pushed the 1010_CFADs branch 6 times, most recently from 0932ff7 to 0083d51 Compare January 24, 2025 16:52
@daflack daflack force-pushed the 1010_CFADs branch 2 times, most recently from ca23bf8 to db52372 Compare February 3, 2025 11:55
@daflack daflack force-pushed the 1010_CFADs branch 2 times, most recently from 478851a to d3c5f25 Compare February 17, 2025 13:09
@daflack
Copy link
Contributor Author

daflack commented Aug 4, 2025

Will require a scientific review and a technical review. Notes for reviewers: pending the re-write of plot.py it was viewed as sensible to not include the plotting of this function. Instead please just focus on whether it would create a CFAD or not.

@daflack daflack changed the title Adds Contour Frequency by Altitude Diagrams (CFAD) Adds Contour Frequency by Altitude Diagrams (CFAD) calculation code Aug 4, 2025
@daflack daflack marked this pull request as ready for review August 4, 2025 10:52
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.

The main thing is to check that the cube name is being set correctly. Otherwise its largely nits from a technical point of view.

It would be good to also have a science review, as I'm not familiar with CFADs.

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.

We should probably also remove the standard name if we are renaming. Otherwise looks good technically.

@jfrost-mo jfrost-mo dismissed their stale review August 12, 2025 08:44

Changes addressed.

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. Now over to the scientific review.

@daflack daflack force-pushed the 1010_CFADs branch 2 times, most recently from 4073fd6 to ca47404 Compare September 8, 2025 12:53
@daflack daflack force-pushed the 1010_CFADs branch 2 times, most recently from ebd84ed to eec0f3b Compare September 24, 2025 13:48
@daflack daflack force-pushed the 1010_CFADs branch 2 times, most recently from 533c8ed to e9ed89e Compare October 2, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Contour Frequency Altitude Diagrams

3 participants