Handle cartopy assets when they cannot be found#2139
Conversation
iluise
left a comment
There was a problem hiding this comment.
thanks for adding this feature. I left a few comments
config/evaluate/eval_config.yml
Outdated
| @@ -1,5 +1,8 @@ | |||
| #optional: if commented out all is taken care of by the default settings | |||
| # NB. global options apply to all run_ids | |||
| global_plotting_options: | |||
There was a problem hiding this comment.
can you comment this out as all global_plotting_options?
It should not be enabled by default.
| "fig_size": global_plotting_opts.get("fig_size", (8, 10)), | ||
| "fps": global_plotting_opts.get("fps", 2), | ||
| "regions": global_plotting_opts.get("regions", ["global"]), | ||
| "cartopy_offline": global_plotting_opts.get("cartopy_offline", True), |
There was a problem hiding this comment.
the default should be that the coastlines are always plotted. Same above
There was a problem hiding this comment.
Correct me if I am wrong, but online downloading of cartopy isn't possible. If this is set to False, the whole point of the feature goes away.
| cartopy.config["pre_existing_data_dir"] = str(work_dir) | ||
| os.environ["CARTOPY_DATA_DIR"] = str(work_dir) | ||
|
|
||
| def _install_cartopy_download_warning_hook() -> None: |
There was a problem hiding this comment.
do we need all these lines for just a warning?
Can we remove them or condense them into the bare minimum?
(functions defined within functions should be avoided)
|
@iluise Thanks for reviewing this PR, I added the requested changes except for one, which I think should be kept. Please let me know what you think. |
- removed cartopy offline flag - renamed the function that disables cartopy download
- removed misleading logging message - added try except to catch coastlines when it fails
Description
A script was added to
packages/evaluate/src/weathergen/evaluate/plotting/plotter.pyto catch when cartopy tries to download missing asset.If cartopy is not found, global plots will look like this:
Issue Number
Closes #2071
Is this PR a draft? Mark it as draft.
Checklist before asking for review
./scripts/actions.sh lint./scripts/actions.sh unit-test./scripts/actions.sh integration-testlaunch-slurm.py --time 60