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

Improve the Python example diagnostic and documentation #1827

Merged
merged 17 commits into from
Oct 16, 2020

Conversation

bouweandela
Copy link
Member

@bouweandela bouweandela commented Sep 18, 2020

Make the example recipe and python diagnostic a bit nicer

  • Add documentation on the example Python recipe
  • Demonstrate plotting a timeseries in addition to a map
  • Use CMIP6 as well as CMIP5 data in one recipe
  • Reduce amount of data needed (now 2 files of about 60 MB each).
  • Already anticipates the move of output_file_type/write_plots/write_netcdf from config-user.yml to those recipes that need it in Deprecate defining write_plots and write_netcdf in config-user file ESMValCore#808
  • Updates the intersphinx URLs

This will allow using the example Python diagnostic to create the timeseries in the tutorial too.


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

Related to ESMValGroup/ESMValTool_Tutorial#141

@bouweandela
Copy link
Member Author

Do we need documentation for the example recipes?

@Peter9192
Copy link
Contributor

Do we need documentation for the example recipes?

I think that would be nice. I for one have looked for it a few times.

@Peter9192
Copy link
Contributor

Do we need documentation for the example recipes?

I'd like to use this recipe in the tutorial. Therefore, it would be nice to be able to point to some basic documentation as well.

@Peter9192
Copy link
Contributor

@bouweandela some of the changes in this PR seem to be unrelated to the objective. Is this intended?

@bouweandela
Copy link
Member Author

bouweandela commented Oct 14, 2020

some of the changes in this PR seem to be unrelated to the objective. Is this intended?

@Peter9192 Yes, I think so, the objective of the pull request is to improve the documentation on how to make your first recipe and diagnostic.

@bouweandela bouweandela changed the title Make example python recipe a bit nicer Improveme the Python example diagnostic and documentation Oct 14, 2020
@bouweandela bouweandela marked this pull request as ready for review October 14, 2020 14:55
@bouweandela
Copy link
Member Author

@schlunma I already implemented the changes needed to deprecate output_file_type etc in ESMValGroup/ESMValCore#808, but that didn't make it into the ESMValCore v2.1 release, so hopefully it will make it in v2.2. To try out the new default without an output_file_type option in config-user, where all plots of a certain type are written, you can set it to null in config-user.yml. Because the value in config-user.yml overwrites whatever you set in the recipe at the moment, specifying this from the recipe doesn't work yet.

@bouweandela bouweandela changed the title Improveme the Python example diagnostic and documentation Improve the Python example diagnostic and documentation Oct 14, 2020
Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

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

Good job @bouweandela, works as expected (tested with output_file_type: png and output_file_type: null). I have some comments regarding the new helper functions.

esmvaltool/diag_scripts/shared/_base.py Show resolved Hide resolved
esmvaltool/diag_scripts/shared/_base.py Outdated Show resolved Hide resolved
esmvaltool/diag_scripts/shared/_base.py Show resolved Hide resolved
esmvaltool/diag_scripts/shared/_base.py Show resolved Hide resolved
esmvaltool/diag_scripts/shared/_base.py Outdated Show resolved Hide resolved
esmvaltool/diag_scripts/shared/_base.py Show resolved Hide resolved
Co-authored-by: Manuel Schlund <32543114+schlunma@users.noreply.github.com>
Co-authored-by: Manuel Schlund <32543114+schlunma@users.noreply.github.com>
Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

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

I am happy now, nice work! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants