Skip to content

Support transect difference plotting and aggregation#2026

Merged
James Warner (jwarner8) merged 50 commits into
mainfrom
2025_transect_diff
Apr 15, 2026
Merged

Support transect difference plotting and aggregation#2026
James Warner (jwarner8) merged 50 commits into
mainfrom
2025_transect_diff

Conversation

@jwarner8
Copy link
Copy Markdown
Contributor

@jwarner8 James Warner (jwarner8) commented Apr 7, 2026

Addresses #2025

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.
  • Ensure rose-suite.conf.example has been updated if new diagnostic added.
  • 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.

@jwarner8 James Warner (jwarner8) added the enhancement New feature or request label Apr 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Coverage

@jwarner8
Copy link
Copy Markdown
Contributor Author

Transect differences working (with aggregation), but adding some more code to enforce the ordering of pressure levels in case they are defined different in UM/ML models, like this...
image

@jwarner8
Copy link
Copy Markdown
Contributor Author

Now much better, likely linked to #1535 too (threaded through for all transect recipes).
image

@jwarner8
Copy link
Copy Markdown
Contributor Author

All options now working with separate sections in webpage, have also changed alpha of inset so that it doesn't cover all of the plot details.
image

This is still well visible when the anomalies are small, and white is used

image

@jwarner8
Copy link
Copy Markdown
Contributor Author

James Warner (jwarner8) commented Apr 7, 2026

TODO

  • Update code comments, tidy
  • Figure out why aggregate difference by hour of day isn't working, and whether we need to ensure _extract_common_time_points first? As models have different output frequencies. Solution: wasn't using forecast_period.
  • Ensure GUI is descriptive
  • Add additional recipes for other aggregation difference types (lead-time, valid-time, hour of day).
  • Combine increasing levels with existing misc check/functionality (decided against)
  • Py testing!!! ;-)

Tests required...

  • _slice_cube_on_common_levels (check that it returns correct subset)
  • extract_common_pressure_levels (ensure that only a CubeList containing two cubes is passed into function.
  • extract_common_pressure_levels (if cube without pressure is tried, it should return ValueError.
  • extract_common_pressure_levels (generic test to check that it returns two cubes with common pressure levels, remove one slice from test cube and then see if we can equate pressure points after going through function).
  • extract_common_pressure_levels (check that if there are no matching levels it returns a ValueError, can manually change a cube to test this).

Comment thread src/CSET/operators/misc.py Outdated
@jwarner8 James Warner (jwarner8) marked this pull request as ready for review April 9, 2026 09:43
@jwarner8 James Warner (jwarner8) changed the title Support transect difference plotting Support transect difference plotting and aggregation Apr 13, 2026
Copy link
Copy Markdown
Collaborator

@daflack David Flack (daflack) left a comment

Choose a reason for hiding this comment

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

Looking good, one typo and requests for some more testing.

  1. Can we get a test(s) that cover line 309-312 in misc.pycurrently all tests skip these lines.
  2. Can we increase coverage in transect.py - needs tests for L136 (if statement false), L165 - to test the exception works, and L174 is currently always true - if expected can be an else, if not and something happens (even if yet to be implemented) when statement is false can this be tested as well please.

Haven't run it yet, but otherwise is looking great.

Comment thread src/CSET/operators/misc.py Outdated
@jwarner8
Copy link
Copy Markdown
Contributor Author

James Warner (jwarner8) commented Apr 14, 2026

Looking good, one typo and requests for some more testing.

  1. Can we get a test(s) that cover line 309-312 in misc.pycurrently all tests skip these lines.
  2. Can we increase coverage in transect.py - needs tests for L136 (if statement false), L165 - to test the exception works, and L174 is currently always true - if expected can be an else, if not and something happens (even if yet to be implemented) when statement is false can this be tested as well please.

Haven't run it yet, but otherwise is looking great.

  • Updated typo
  • Add test for misc if plevs are wrong way round in one cube
  • Fix transect L136 to else (was bug before), and tested in plotting.
  • L165 test case for longitude, and ensure this is used as output transect coordinate. Links to 174 always true

@jwarner8
Copy link
Copy Markdown
Contributor Author

Now correctly switches to longitude on x axis when this varies more than latitude
image

@jwarner8
Copy link
Copy Markdown
Contributor Author

James Warner (jwarner8) commented Apr 15, 2026

Test coverage now 100% for transect.py, ready for final review.

Copy link
Copy Markdown
Collaborator

@daflack David Flack (daflack) left a comment

Choose a reason for hiding this comment

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

Happy with these changes, thanks for adding the extra tests.

@jwarner8 James Warner (jwarner8) merged commit 21b9202 into main Apr 15, 2026
8 checks passed
@jwarner8 James Warner (jwarner8) deleted the 2025_transect_diff branch April 15, 2026 12:59
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.

3 participants