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

Restructure ClimWIP #1919

Merged
merged 9 commits into from
Nov 26, 2020
Merged

Restructure ClimWIP #1919

merged 9 commits into from
Nov 26, 2020

Conversation

lukasbrunner
Copy link
Contributor

@lukasbrunner lukasbrunner commented Nov 25, 2020

The original ClimWIP recipe was growing a bit big, and for upcoming work we need to make it even bigger. Therefore, we suggest a restructuring to the following basic format. This does not modify any of the existing code, but only moves it around.

esmvaltool/
    weighting/
        climwip/
            io_functions.py
            core_functions.py
            main.py
        plot_utilities.py
        weighted_temperature_graph.py
        weighted_temperature_map.py

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

Modified recipe/diagnostic

  • Update documentation for the recipe to the doc/sphinx/source/recipes folder
  • Update provenance information if needed
  • Assign the author(s) of the affected recipe(s) as reviewer(s)

Closes #1918

@lukasbrunner lukasbrunner added diagnostic EUCP www.eucp-project.eu labels Nov 25, 2020
@lukasbrunner lukasbrunner self-assigned this Nov 25, 2020
@lukasbrunner lukasbrunner marked this pull request as ready for review November 25, 2020 16:34
@lukasbrunner
Copy link
Contributor Author

This should do the trick I think @Peter9192 Do we also need a scientific review for this?

@Peter9192
Copy link
Contributor

@esmvalbot please run recipe_climwip.yml

@esmvalbot
Copy link

esmvalbot bot commented Nov 25, 2020

Since @Peter9192 asked, ESMValBot will run recipe recipe_climwip.yml as soon as possible, output will be generated here

@Peter9192
Copy link
Contributor

This should do the trick I think @Peter9192 Do we also need a scientific review for this?

Maybe we can both briefly inspect the output of the bot, to see if it still looks okay.
And notify @ruthlorenz that she should update the path the to climwip script in her recipe?

@Peter9192
Copy link
Contributor

I updated the top post to reflect the main changes in this PR.

@esmvalbot
Copy link

esmvalbot bot commented Nov 25, 2020

ESMValBot is happy to report recipe recipe_climwip.yml ran OK, output has been generated here

@lukasbrunner
Copy link
Contributor Author

ESMValBot is happy to report recipe recipe_climwip.yml ran OK, output has been generated here

looks identical to my output from before the changes :)

@bettina-gier bettina-gier merged commit d835493 into master Nov 26, 2020
@bettina-gier bettina-gier deleted the climwip_restructure branch November 26, 2020 13:15
@zklaus
Copy link

zklaus commented Nov 26, 2020

There are a few outstanding code quality issues. Probably too early to merge.

@ruthlorenz
Copy link
Contributor

I depend on it and its not the end of the story, there will be follow up pull requests ;-) we can deal with the codacy issues later

@zklaus
Copy link

zklaus commented Nov 26, 2020

Promise? 😛

@ruthlorenz
Copy link
Contributor

if possible yes ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostic EUCP www.eucp-project.eu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restructure ClimWIP files
5 participants