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.

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

Diagnostic for recipe_eyring13jgr.yml Fig. 12 #1922

Merged
merged 19 commits into from Dec 16, 2020
Merged

Conversation

LisaBock
Copy link
Contributor

@LisaBock LisaBock commented Nov 26, 2020

Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • Give this pull request a descriptive title that can be used as a one line summary in a changelog
  • Make sure your code is composed of functions of no more than 50 lines and uses meaningful names for variables
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Preferably Codacy code quality checks pass, however a few remaining hard to solve Codacy issues are still acceptable. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes

New recipe/diagnostic

  • Add documentation for the recipe to the doc/sphinx/source/recipes folder and add a new entry to index.rst
  • Add provenance information

Closes #1921

@hb326 hb326 added this to In Progress in November 2020 Nov 26, 2020
Some code cleanup.
@hb326
Copy link
Contributor

hb326 commented Dec 1, 2020

I started to clean up the code of the diagnostic and the recipe a little. I also ran the recipe with a subset of CMIP5 and CMIP6 data, and it works for both.

@LisaBock LisaBock marked this pull request as draft December 3, 2020 08:40
esmvaltool/recipes/recipe_eyring13jgr_12.yml Show resolved Hide resolved
esmvaltool/recipes/recipe_eyring13jgr_12.yml Show resolved Hide resolved
esmvaltool/recipes/recipe_eyring13jgr_12.yml Show resolved Hide resolved
esmvaltool/recipes/recipe_eyring13jgr_12.yml Show resolved Hide resolved
esmvaltool/recipes/recipe_eyring13jgr_12.yml Show resolved Hide resolved
esmvaltool/recipes/recipe_eyring13jgr_12.yml Show resolved Hide resolved
@@ -6,21 +6,25 @@ Diagnostics of stratospheric dynamics and chemistry
Overview
--------

Copy link
Contributor

Choose a reason for hiding this comment

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

I would have thought that we would have a totally different entry to the documentation for the diagnostics from Eyring 13 JGR. At the moment it is listed as something from the Eyring 06 JGR paper... There are also two more diagnostics from Eyring 13 JGR waiting to be fianlized and merged, so it kind of makes sense to have a different entry for the diagnostics of this paper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I agree! I seperated the documentations...

@LisaBock
Copy link
Contributor Author

I have to rerun the recipe once more because their was one CMIP6 model with strange outliers.
If that is successfull we could finalize the pull request...

@LisaBock LisaBock marked this pull request as ready for review December 14, 2020 12:34
@LisaBock
Copy link
Contributor Author

The pull request is ready for a final review!

Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

Everything works fine for me 👍 This is ready to merge.

e13fig12_exp_MMM: "historical"
e13fig12_season: "DJF"
e13fig12_multimean: true
#
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for this # at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. This is a mistake. I will delete it.

@hb326 hb326 moved this from In Progress to In Review in November 2020 Dec 16, 2020
Implementation of a few corrections to the diagnostics documentation
Copy link
Contributor

@hb326 hb326 left a comment

Choose a reason for hiding this comment

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

Looks ok to me now.

@hb326 hb326 merged commit da9521a into master Dec 16, 2020
November 2020 automation moved this from In Review to Merged Dec 16, 2020
@hb326 hb326 deleted the version2_eyring13jgr branch December 16, 2020 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

adding eyring13jgr Fig. 12
5 participants