Radiation budget: labelling plots with alias#4292
Conversation
NParsonsMO
changed the title
|
Output before change: |
|
Output after change: |
|
@ehogan I'm not sure if I should change the picture in the documentation? Or mention |
|
@ehogan yet again GH notifies me of a PR that's being created by a MO bod, though I'm neither assigned nor requested as a reviewer - heck, am not even of an owner of the code that's being changed; I noticed this previously with your guys' work on RTW, do you folks at the MO have a config setting that says "ping V everytime we open a PR"? 🤣 |
🤣 Not to my knowledge! 🤪 I can't see why you would have been notified?! 🤯 |
| @@ -294,6 +294,7 @@ def plot_data( | |||
| model_dataset, | |||
There was a problem hiding this comment.
model_dataset is now no longer used in plot_data; would it be possible to remove it, please? 😊
There was a problem hiding this comment.
I also noticed that I hadn't added the new variable to the docstring, so have now done so:
| all_model_data = derive_additional_variables(unordered_model_data) | ||
| model_data = order_data(all_model_data, obs_names, obs_unit) | ||
| model_period = f"{group[0]['start_year']} - {group[0]['end_year']}" | ||
| model_label = group[0]["alias"] |
There was a problem hiding this comment.
If I remember correctly, we wanted to affect alias via CMEW by adding alias to a given dataset in the recipe. I'm debating whether we should, now that you have shared before and after images showing the introduction of the alias, specify the alias in the dataset so that the images are the same as before? 🤔
There was a problem hiding this comment.
Also, and this has nothing to do with what you have done, but group[0] is a bit dubious. An assumption has been made that the start_year and end_year from the first variable in the list (for a given dataset) is the same as all the others. This might be a reasonable assumption to make! We should either add a comment stating this assumption, or do a check to ensure that these values are consistent across variables (they really should be, and I don't know what we would do if they weren't!).
There was a problem hiding this comment.
If I remember correctly, we wanted to affect
aliasvia CMEW by addingaliasto a givendatasetin the recipe. I'm debating whether we should, now that you have shared before and after images showing the introduction of thealias, specify thealiasin thedatasetso that the images are the same as before? 🤔
There was a problem hiding this comment.
Also, and this has nothing to do with what you have done, but
group[0]is a bit dubious. An assumption has been made that thestart_yearandend_yearfrom the first variable in the list (for a given dataset) is the same as all the others. This might be a reasonable assumption to make! We should either add a comment stating this assumption, or do a check to ensure that these values are consistent across variables (they really should be, and I don't know what we would do if they weren't!).
Done within the function:
Do you want it documented elsewhere as well?
Good question. I wonder whether it's worth adding something to ESMValTool documentation: Radiation Budget: User settings in recipe to mention that |
Co-authored-by: Emma Hogan <ehogan@users.noreply.github.com>
I put it in "User settings in recipe". |
3 participants
Changing the f-strings in the plot titles to use a dataset's alias instead of its name.
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.
New or updated recipe/diagnostic
New or updated data reformatting script
To help with the number of pull requests: