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.

Sign up for GitHub

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

Jump to bottom

Support flexible pressure_level dependent colourbars #1140

Merged
merged 7 commits into from
Feb 14, 2025
Merged

Support flexible pressure_level dependent colourbars #1140

merged 7 commits into from
Feb 14, 2025

Conversation

Sylviabohnenstengel
Copy link
Contributor

@Sylviabohnenstengel Sylviabohnenstengel commented Feb 12, 2025 •
edited by jfrost-mo
Loading

Fixes #1139 and parts of #949

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.

@Sylviabohnenstengel Sylviabohnenstengel mentioned this pull request Feb 12, 2025
Closed
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 12, 2025 •
edited
Loading

Coverage

@Sylviabohnenstengel
Copy link
Contributor Author

not ready for review yet

@Sylviabohnenstengel
Copy link
Contributor Author

now working for predefined and not predefined pressure levels for lfric data.
Not yet working for UM data as the UM cube has the wrong standard name for temperature on pressure levels. Awaiting fix from @jfrost-mo

@Sylviabohnenstengel
Copy link
Contributor Author

finetuning the predefined pressure level dependent min and max values for temperature in Kelvin. Doing this based on spatially averaged profiles.

We require further additions for other main variables and levels.

@Sylviabohnenstengel Sylviabohnenstengel self-assigned this Feb 13, 2025
@Sylviabohnenstengel
Copy link
Contributor Author

Sylviabohnenstengel commented Feb 13, 2025 •
edited
Loading

@jfrost-mo @jwarner8
This PR is now working for LFRic data. For Um data it needs a fix #1141
We decided to deal with #1141 first and then rebase this PR as they deal with the same code logic.

If we want to add more finetuned variables per pressure level maybe @BernardClaxton can make those changes to the colorbar_map.json?

@jfrost-mo jfrost-mo force-pushed the 949_populate_colorbar_json_file_with_environment_variable branch from 000c108 to 60e4f97 Compare February 14, 2025 11:21
@jfrost-mo jfrost-mo changed the base branch from main to colourbar_name_fallback February 14, 2025 11:21
@jfrost-mo jfrost-mo changed the title add flexible pressure_level dependent colourbars Support flexible pressure_level dependent colourbars Feb 14, 2025
@jfrost-mo
Copy link
Member

Has now been rebased, though will still need some tests.

@jfrost-mo
Copy link
Member

Tests are now implemented and this PR is now ready for review.

@jfrost-mo jfrost-mo force-pushed the 949_populate_colorbar_json_file_with_environment_variable branch from 8d4b82d to ec47229 Compare February 14, 2025 12:08
@jfrost-mo jfrost-mo marked this pull request as ready for review February 14, 2025 12:08
@jfrost-mo jfrost-mo requested a review from jwarner8 February 14, 2025 12:13
@jwarner8 jwarner8 mentioned this pull request Feb 14, 2025
Merged
7 tasks
jwarner8
jwarner8 approved these changes Feb 14, 2025
View reviewed changes
Copy link
Contributor

@jwarner8 jwarner8 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

src/CSET/operators/plot.py Outdated Show resolved Hide resolved
Sylviabohnenstengel and others added 5 commits February 14, 2025 17:57
@Sylviabohnenstengel @jfrost-mo
Allow pressure_level dependent colorbar
5902dc6 
Only includes the colorbar definitions for temperature. More will be
added later.

Fixes #1139
@jfrost-mo
Try long name before standard name in colorbar selection
3cb2558 
The long name is the one we normalise, so will be more likely to have
a colorbar definition.
@jfrost-mo
Test pressure level specific colorbar entry
9798651
@jfrost-mo
Fix fencepost problem with colorbar bins
89911b6 
This makes the numbers on the colorbar labels much rounder if the
range is a multiple of 20.

This comes from the fencepost problem, which describes how you need
n+1 dividers to support n bins. For example, imagine a fence with 5
segments, like so:

    |###|###|###|###|###|

Notice that we have 6 posts (denoted with | characters) for our 5
fence segments. This is the fence post problem.
@jfrost-mo
Update _colorbar_map_levels docstring
c8ee1f1
@jfrost-mo jfrost-mo force-pushed the 949_populate_colorbar_json_file_with_environment_variable branch from 38e4b4a to c8ee1f1 Compare February 14, 2025 17:59
Base automatically changed from colourbar_name_fallback to main February 14, 2025 18:01
@jfrost-mo jfrost-mo force-pushed the 949_populate_colorbar_json_file_with_environment_variable branch from 33e109c to 97e0fd4 Compare February 14, 2025 18:07
@jfrost-mo jfrost-mo merged commit fb3752b into main Feb 14, 2025
8 checks passed
@jfrost-mo jfrost-mo deleted the 949_populate_colorbar_json_file_with_environment_variable branch February 14, 2025 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfrost-mo jfrost-mo jfrost-mo left review comments

@jwarner8 jwarner8 jwarner8 approved these changes

Assignees

@Sylviabohnenstengel Sylviabohnenstengel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Colorbars for variables on pressure levels
3 participants
@Sylviabohnenstengel @jfrost-mo @jwarner8