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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

More removals of instances of write_plots from Python diagnostics (appears to be the final removal from Py diags) #2394

Merged
merged 16 commits into from Oct 29, 2021

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Oct 29, 2021

Description


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 馃洜 Technical or 馃И Scientific review.

@@ -49,7 +49,7 @@ def get_provenance_record(gatt, vatt, ancestor_files):
def main(cfg):
"""Ensemble Clustering Diagnostics."""
out_dir = cfg['work_dir']
write_plots = cfg['write_plots']
write_plots = 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 this needed? I would remove it and see if the diagnostic still works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah could do, didn't want to temper too much with the diag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, tis history now 馃彺鈥嶁槧锔

Copy link
Contributor

@zklaus zklaus left a comment

Choose a reason for hiding this comment

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

Sorry, changed my mind a bit. Let's add the remaining diags as well. More comments to come.

@zklaus
Copy link
Contributor

zklaus commented Oct 29, 2021

The seaice feedback diagnostic uses the constant from here---which probably should go as well.

@zklaus
Copy link
Contributor

zklaus commented Oct 29, 2021

I'll tackle the R diagnostics separately, if you also address here

  • recipe_carvalhais14nat
  • recipe_runoff_et (write_netcdf)
  • recipe_seaice_feedback
  • remove the constants from diag_scripts.shared.names

Deal?

@valeriupredoi
Copy link
Contributor Author

OK will do but I don't speak NCL so that's why I'm looking only at 馃悕 diags

@zklaus
Copy link
Contributor

zklaus commented Oct 29, 2021

Aren't all the ones I listed snakey?

@valeriupredoi
Copy link
Contributor Author

yesh they are, on it now, bud!

@valeriupredoi
Copy link
Contributor Author

OK done! removed all write_plots, write_netcdf, WRITE_PLOTS, WRITE_NETCDF variables (there is one func only but that's diag custom, not messing with it, in esmvaltool/diag_scripts/shapeselect/diag_shapeselect.py) from 馃悕 diags 馃嵑

@valeriupredoi valeriupredoi changed the title More removals of instances of write_plots from Python diagnostics More removals of instances of write_plots from Python diagnostics (appears to be the final removal from Py diags) Oct 29, 2021
Copy link
Contributor

@zklaus zklaus left a comment

Choose a reason for hiding this comment

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

Nice!

@valeriupredoi
Copy link
Contributor Author

thanks dude, not the most pleasant PR to review 馃槅

@valeriupredoi valeriupredoi merged commit 808e5c9 into main Oct 29, 2021
@valeriupredoi valeriupredoi deleted the more_removals_write_plots branch October 29, 2021 13:43
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.

None yet

2 participants