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.

Sign up for GitHub

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

Jump to bottom

Various changes (unit tests, scripts from private repositories) #1012

Merged
merged 17 commits into from
May 15, 2019
Merged

Various changes (unit tests, scripts from private repositories) #1012

merged 17 commits into from
May 15, 2019

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Apr 7, 2019 •
edited
Loading

This PR contains several changes:

  • Unit tests for esmvaltool.diag_scripts.shared.io and esmvaltool.diag_scripts.shared.iris_helpers
  • Fix and derivation scripts from the private repositories which should be available in the public repo
  • Various smaller optimizations
  • Update of recipe_flato13ipcc with recent CMIP6 models.

@schlunma schlunma self-assigned this Apr 7, 2019
@schlunma schlunma requested a review from bouweandela April 7, 2019 18:06
@axel-lauer axel-lauer mentioned this pull request Apr 8, 2019
Merged
@bouweandela bouweandela requested a review from jvegreg April 17, 2019 14:55
bouweandela
bouweandela requested changes May 6, 2019
View reviewed changes
Copy link
Member

@bouweandela bouweandela left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please see the comments below. Also, I think it is more important to add unit tests for the preprocessor (i.e. fixes and derive functions) than for diagnostic code, so it would be good if you could add that too.

esmvaltool/diag_scripts/shared/iris_helpers.py Outdated
ValueError
Given coordinate differs for the input cubes.

"""
coord = None
if isinstance(cubes, dict):
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here

@schlunma
Copy link
Contributor Author

schlunma commented May 6, 2019

I addressed all comments, added units tests for the preprocessor and fix files and removed the fixes for the CMIP6 models (see discussion in #983).

bouweandela
bouweandela approved these changes May 9, 2019
View reviewed changes
Copy link
Member

@bouweandela bouweandela left a comment

Choose a reason for hiding this comment

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

Code looks good, but I have not tested running this.

@mattiarighi mattiarighi merged commit d0faaba into version2_development May 15, 2019
@mattiarighi mattiarighi deleted the version2_diag_scripts_shared_tests branch May 15, 2019 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bouweandela bouweandela bouweandela approved these changes

@jvegreg jvegreg Awaiting requested review from jvegreg

Assignees

@schlunma schlunma

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schlunma @bouweandela @mattiarighi