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

Removed write_plots and write_netcdf in some python diagnostics #2036

Merged
merged 2 commits into from
Feb 17, 2021

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Feb 17, 2021

This PR deprecates the options write_plots and write_netcdf for some python diagnostics (the ones I coded) and for some tests. I successfully tested all affected recipes after the change.


Before you get started

Checklist

It is the responsibility of the author to make sure the PR is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@schlunma schlunma added this to the v2.2.0 milestone Feb 17, 2021
@schlunma schlunma requested a review from bouweandela February 17, 2021 08:38
@schlunma schlunma self-assigned this Feb 17, 2021
@schlunma schlunma changed the title Deprecated write_plots and write_netcdf in some diagnostics and config-user.yml Deprecated write_plots and write_netcdf in some python diagnostics Feb 17, 2021
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 changes look good to me, but I do not have time to test these recipes. Do you think we could run some of them with the @esmvalbot? If not, do you think it would be OK to merge this as is, since we plan to test recipes for the release anyway?

@bouweandela
Copy link
Member

Would it make sense to replace the word 'Deprecated' in the title with the word 'Remove'?

It would be good if you could mention in the pull request description which recipes are affected by these changes, that will make it easier to find where things went wrong if something turns out to be broken later.

@schlunma schlunma changed the title Deprecated write_plots and write_netcdf in some python diagnostics Removed write_plots and write_netcdf in some python diagnostics Feb 17, 2021
@schlunma
Copy link
Contributor Author

Code changes look good to me, but I do not have time to test these recipes. Do you think we could run some of them with the @esmvalbot? If not, do you think it would be OK to merge this as is, since we plan to test recipes for the release anyway?

I tested all recipes, but I will run some (which do not require lots of memory) with the bot. So merging should be safe.

Would it make sense to replace the word 'Deprecated' in the title with the word 'Remove'?

Done

It would be good if you could mention in the pull request description which recipes are affected by these changes, that will make it easier to find where things went wrong if something turns out to be broken later.

This is the list:

  • recipe_ecs.yml
  • recipe_tcr.yml
  • recipe_cox18nature.yml
  • recipe_flato13ipcc.yml
  • recipe_schlund20jgr_*.yml

@bouweandela did you see this comment?

@schlunma
Copy link
Contributor Author

@esmvalbot Please run recipe_cox18nature.yml

@esmvalbot
Copy link

esmvalbot bot commented Feb 17, 2021

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

@schlunma
Copy link
Contributor Author

@esmvalbot Please run recipe_tcr.yml

@esmvalbot
Copy link

esmvalbot bot commented Feb 17, 2021

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

@schlunma
Copy link
Contributor Author

@esmvalbot Please run recipe_ecs.yml

@esmvalbot
Copy link

esmvalbot bot commented Feb 17, 2021

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

@esmvalbot
Copy link

esmvalbot bot commented Feb 17, 2021

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

@esmvalbot
Copy link

esmvalbot bot commented Feb 17, 2021

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

@esmvalbot
Copy link

esmvalbot bot commented Feb 17, 2021

ESMValBot is sorry to report it failed to run recipe recipe_ecs.yml: exit is 1, output has been generated here

@schlunma
Copy link
Contributor Author

The bot failed due to missing data, but the recipe ran fine on my machine

@schlunma schlunma merged commit d2af92f into master Feb 17, 2021
@schlunma schlunma deleted the deprecate_write_plots_write_netcdf branch February 17, 2021 12:51
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.

2 participants