-
Notifications
You must be signed in to change notification settings - Fork 44
Align custom lwp table with CMIP6 version
#2791
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2791 +/- ##
=======================================
Coverage 95.45% 95.45%
=======================================
Files 260 260
Lines 15496 15496
=======================================
Hits 14792 14792
Misses 704 704 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
axel-lauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and solves a problem with multi_model_statistics that prevented using datasets that provide lwp directly and datasets that require derivation of lwp.
|
@ESMValGroup/esmvaltool-coreteam This introduces a (small) backwards-incompatible change. However, I don't expect any problems with this. Please object if you have any concerns about this. This will be merged in 2 weeks. |
valeriupredoi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all good by me, putting a temporary blocker to allow for the two weeks and avoid eager merges; I put "Merge PR 2791" in me agenda, so I won't forget 😁
|
this will be merged this week, Friday 15th 🍺 @ESMValGroup/esmvaltool-coreteam please see #2791 (comment) and comment here as per your one and a half pennies (British two cents 😁 ) |
|
time's up! I'm merging this 🚚 |
Description
This PR aligns our custom
lwptable with the CMIP6 version.See #2789 (review).
Backwards-incompatible change #
This PR also changes standard name of
lwpin our custom table from""to"atmosphere_mass_content_of_cloud_liquid_water", which will be a problem with downstream code that depends on thestandard_nameentry. I won't expect any problems with this to be honest.There is no way to restore the old behavior. Downstream code needs to be adapted.
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
To help with the number pull requests: