-
Notifications
You must be signed in to change notification settings - Fork 128
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
Changes in shared scripts for Schlund et al., JGR: Biogeosciences, 2020 #1845
Conversation
dict_['var_name'] = dict_.pop('short_name') | ||
return dict_ | ||
|
||
|
||
def iris_project_constraint(projects, cfg, negate=False): | ||
"""Create `iris.Constraint` to select specific projects from data. | ||
def get_mean_cube(datasets): |
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.
This looks very close to the multi_model_statistics
preprocessor function, do we need an extra function here?
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.
Yes, this is similar to multi_model_statistics
but way easier. Also, multi_model_statistics
takes products
as input, which is inconvenient in this case. I'd like to keep that.
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.
Multi model statistics also supports cubes since a few months. Actually, I hope that multi model statistics will soon look like this function, once we have lazy vertical interpolation I think it can be implemented like that. However, it's fine if you want to keep it like this, we can always replace this later. When I was first reviewing this, I thought it was part of the public API, but it isn't, so it should be fine to add and remove things.
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! Are the shared functions that are changed in this pull request also used in other diagnostics? If so, it might be good to check that those aren't broken by your changes.
Thanks for your review! I check if these functions are used in other diagnostics and test all those. |
Succesfully tested |
This PR adds changes to shared diagnostic script functions necessary for including my recently accepted paper on ("Constraining uncertainty in projected gross primary production with machine learning"). It would be really nice to get this merged as fast as possible so I can open the PR with the actual additions after that. I would really like to have this in v2.1 so I can update the "code availability" section of my paper 😄
Tasks
yamllint
to check that your YAML files do not contain mistakesRelated to #1844