Combined `offline` and `always_search_esgf` into a single option `search_esgf` #1935

schlunma · 2023-02-17T14:32:34Z

Description

Deprecation

This PR adds a new config option search_esgf, which replaces the old options offline and always_search_esgf. offline has been deprecated (scheduled for removal in v2.10.0), always_search_esgf is simply removed (has not been included in a release yet). The following table shows the usage of the new option:

Old `offline`	Old `always_search_esgf`	New `search_esgf`	Description
`False`	`False`	`'when_missing'`	Use local files when possible (regardless of version), otherwise download
`False`	`True`	`'always'`	Use local files only if latest version is available, otherwise download
`True`	`False`	`'never'`	Only use local files, never download
`True`	`True`	`'never'`	Only use local files, never download

In addition, this PR cleans and unifies the code to deprecate configuration options in order to allow changing one configuration option (e.g., search_esgf) based on another (deprecated) option (e.g., offline). In addition, now a deprecation warning is always raised when that option is use. Previously, only a warning was raised when the option was different to the default.

Closes #1931

Link to documentation:

Before you get started

☝ Create an issue to discuss what you are going to do

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.

🧪 The new functionality is relevant and scientifically sound
🛠 This pull request has a descriptive title and labels
🛠 Code is written according to the code quality guidelines
🧪 and 🛠 Documentation is available
🛠 Unit tests have been added
🛠 Changes are backward compatible
🛠 Any changed dependencies have been added or removed correctly
🛠 The list of authors is up to date
🛠 All checks below this pull request were successful

To help with the number pull requests:

🙏 We kindly ask you to review two other open pull requests in this repository

…ecipe

- Use configuration from experimental Output file is now a Path fx file facets are now correctly set when parsing the recipe, no need to do it in data finder anymore

… function

schlunma · 2023-02-28T08:40:19Z

Thanks for picking this up @schlunma! When I run the tool with offline: false in config-user.yml, I get the deprecation warning twice:

/home/bandela/src/esmvalgroup/esmvalcore/esmvalcore/config/_config_validators.py:340: ESMValCoreDeprecationWarning: The configuration option or command line argument `offline` has been deprecated in ESMValCore version 2.8.0 and is scheduled for removal in version 2.10.0. Please use the options `search_esgf=never` (for `offline=True`) or `search_esgf=when_missing` (for `offline=False`) instead. These are exact replacements.
  warnings.warn(deprecation_msg, ESMValCoreDeprecationWarning)
/home/bandela/src/esmvalgroup/esmvalcore/esmvalcore/_config/__init__.py:13: ESMValCoreDeprecationWarning: The private module `esmvalcore._config` has been deprecated in ESMValCore version 2.8.0 and is scheduled for removal in version 2.9.0. Please use the public module `esmvalcore.config` instead.
  warnings.warn(
/home/bandela/src/esmvalgroup/esmvalcore/esmvalcore/config/_config_validators.py:340: ESMValCoreDeprecationWarning: The configuration option or command line argument `offline` has been deprecated in ESMValCore version 2.8.0 and is scheduled for removal in version 2.10.0. Please use the options `search_esgf=never` (for `offline=True`) or `search_esgf=when_missing` (for `offline=False`) instead. These are exact replacements.
  warnings.warn(deprecation_msg, ESMValCoreDeprecationWarning)

This is weird, I am only getting it once (tested with every combination of offline as config file option and command line argument). Due to the way sessions and configs are created the code that issues the warnings runs more than once, but the default setting of the warnings module is to show warnings just once for each location, so I would only expect this warning once.

However, I also noticed to other problems:

The deprecation warnings shown at the very beginning on screen are missing in the log files (regular + debug log). I guess the reasons is that logging is not configured that that stage, but this is really a problem for user who don't use the tool interactively. They will never see this deprecation warning.
I get the deprecation warning about use_legacy_supplementaries even though I am not using this in my config file/command line and the recipe does not include fx_variables:. I will investigate this. EDIT: This has been solved in 156884c.

schlunma · 2023-02-28T12:22:46Z

Thanks for picking this up @schlunma! When I run the tool with offline: false in config-user.yml, I get the deprecation warning twice:

/home/bandela/src/esmvalgroup/esmvalcore/esmvalcore/config/_config_validators.py:340: ESMValCoreDeprecationWarning: The configuration option or command line argument `offline` has been deprecated in ESMValCore version 2.8.0 and is scheduled for removal in version 2.10.0. Please use the options `search_esgf=never` (for `offline=True`) or `search_esgf=when_missing` (for `offline=False`) instead. These are exact replacements.
  warnings.warn(deprecation_msg, ESMValCoreDeprecationWarning)
/home/bandela/src/esmvalgroup/esmvalcore/esmvalcore/_config/__init__.py:13: ESMValCoreDeprecationWarning: The private module `esmvalcore._config` has been deprecated in ESMValCore version 2.8.0 and is scheduled for removal in version 2.9.0. Please use the public module `esmvalcore.config` instead.
  warnings.warn(
/home/bandela/src/esmvalgroup/esmvalcore/esmvalcore/config/_config_validators.py:340: ESMValCoreDeprecationWarning: The configuration option or command line argument `offline` has been deprecated in ESMValCore version 2.8.0 and is scheduled for removal in version 2.10.0. Please use the options `search_esgf=never` (for `offline=True`) or `search_esgf=when_missing` (for `offline=False`) instead. These are exact replacements.
  warnings.warn(deprecation_msg, ESMValCoreDeprecationWarning)

@remi-kazeroni and I found the reason for this! I don't use a user config file called ~/.esmvaltool/config-user.yml, Remi does. I only get this warning once, Remi twice. I guess you have a default config as well @bouweandela?

This has not been introduced with this PR. If you use the main branch you'll also get two deprecation warnings if you use use_legacy_supplementaries in the default config in ~/.esmvaltool/config-user.yml.

I have no idea what's going on there. Since the warning is raised twice from the exact same location, it should only be printed once. If no one has an idea about this, I suggest we simply ignore this. Two warnings is better than no warning.

bouweandela · 2023-02-28T15:34:18Z

The warning was issued twice because it is first issued from the esmvalcore.config module when it the config-user.yml file is loaded when the module is loaded, and then from the emvalcore._main module when the config-user.yml file gets loaded there. The default filter prints a warning once for every module that issues it. I just pushed 5eeed56 that should solve the issue.

Maybe you already know about this, but I found the code

import traceback
traceback.print_stack()

very useful to find out from where the function that raised the deprecation warning was getting called.

bouweandela · 2023-02-28T15:43:16Z

I found a really simple way to print the warnings issued from loading the configuration file again and added it in a05c7d1. Please feel free to not use that if you find it doesn't do a good job.

schlunma · 2023-02-28T16:02:47Z

The warning was issued twice because it is first issued from the esmvalcore.config module when it the config-user.yml file is loaded when the module is loaded, and then from the emvalcore._main module when the config-user.yml file gets loaded there. The default filter prints a warning once for every module that issues it. I just pushed 5eeed56 that should solve the issue.

Ahh, that makes sense. I missed the part about "once per module" 🤦

Unfortunately, your change didn't work for me. I am still getting two warnings with .esmvaltool/config-user.yml, and now even three warnings when I use a command line argument AND a configuration option.

Maybe we should simply use the filter option "once"?

I found a really simple way to print the warnings issued from loading the configuration file again and added it in a05c7d1. Please feel free to not use that if you find it doesn't do a good job.

That worked great for me! Maybe trying to solve this for all deprecation warnings was overly enthusiastic 😃

bouweandela · 2023-02-28T16:11:50Z

Will have another look..

valeriupredoi

review of docstrings, and docs, bud

doc/quickstart/configure.rst

esmvalcore/_main.py

…e into search_esgf_option

valeriupredoi

couple more minor comments from me 🍺

setup.py

esmvalcore/dataset.py

valeriupredoi

you're a ⭐ Manu! Cheers for all this wonderful work - together with @bouweandela I nominate you guys for the Contributors of the Version Award for 2.8 🏆

bouweandela · 2023-02-28T16:58:51Z

Unfortunately, your change didn't work for me. I am still getting two warnings with .esmvaltool/config-user.yml, and now even three warnings when I use a command line argument AND a configuration option.

Maybe we should simply use the filter option "once"?

It only works when you specify no configuration file, i.e. with the command esmvaltool run examples/recipe_python.yml. Unfortunately, I ran out of time to fix this today and I'm not working tomorrow, so please feel free to undo the code changes I made if you think the previous behavior was better.

schlunma · 2023-02-28T17:01:09Z

I suggest we just live with multiple warnings then, I also tried to solve this with action=once but was not succesful.

If there are no further comments I suggest to merge this by noon tomorrow. Thanks everyone for reviewing!!

This reverts commit 5eeed56.

valeriupredoi · 2023-02-28T17:04:47Z

aren't the warnings being generated by different, multiple, tasks? I couldn't care less for multiple warning TBF - if at all, show them more often, maybe somebody decides to pay attention to them 😆

schlunma · 2023-02-28T17:06:10Z

I think they are all issues from the same process (all the reading of the config file happens at the very beginning of the run where we only have a single process as far as I am aware).

remi-kazeroni · 2023-03-01T15:43:51Z

Thanks a lot @schlunma for your nice work and @valeriupredoi and @bouweandela for reviewing it. I would really like to move on with the feature freeze and the release. It looks to me that issue #1931 is fully addressed by this PR. Should you want to improve the handling of the warnings, I would recommend to open an issue based on findings made in this PR to keep track of things.

valeriupredoi · 2023-03-01T15:48:41Z

awesome 🥳

bouweandela added 30 commits September 10, 2021 16:47

First draft of dataset class

12242e6

Merge branch 'main' of github.com:esmvalgroup/esmvalcore into split-r…

2e3b339

…ecipe

Work in progress

f4c2d72

Merge branch 'main' of github.com:esmvalgroup/esmvalcore into split-r…

696c9f2

…ecipe

Fix error in _dataset.py

a116f88

Add ancillary variables

9b28044

Make loading a cube work from the Python API

ba2ba53

Add an integration test for API load

dca7634

Move stuff from recipe to dataset

e5dd013

Some progess

ccf44f1

Restore previous code

1f3ab2f

Make new recipe format for ancillaries work

6496f78

Work in progress

679f674

Progress

55ad481

... is slow

bff4918

Improved addition of ancillary variables in recipe

c5e2430

Merge branch 'main' of github.com:esmvalgroup/esmvalcore into split-r…

e5a1382

…ecipe

Fix things that got broken due to lack of tests

f1e54a4

Remove old config-user.yml code

9e95091

Update data finder tests

3c36ecb

- Use configuration from experimental Output file is now a Path fx file facets are now correctly set when parsing the recipe, no need to do it in data finder anymore

Update CMOR table load tests to use file instead of dict

5d54394

Remove unnessary code

2d19798

Update PreprocessorFile creation and remove test for no longer needed…

8becb37

… function

Work in progress

7ad9c4e

Fix EMAC and ICON tests

c0244a8

Use session instead of config-user

185806b

Fix more tests

dd2cf66

Move experimental.config code to esmvalcore._config

71f6ee4

Fix more tests

c3f6084

Fix more tests

b738a83

Only adapt use_legacy_supplementaries in CFG if necessary

156884c

Avoid repeating deprecation warnings

5eeed56

Print warnings about deprecated config options again after the run

a05c7d1

valeriupredoi reviewed Feb 28, 2023

View reviewed changes

schlunma added 2 commits February 28, 2023 17:22

Improved doc with suggestions by @valeriupredoi

3260d97

Merge branch 'search_esgf_option' of github.com:ESMValGroup/ESMValCor…

54a180f

…e into search_esgf_option

valeriupredoi requested changes Feb 28, 2023

View reviewed changes

setup.py Show resolved Hide resolved

esmvalcore/dataset.py Show resolved Hide resolved

Another doc update by @valeriupredoi

56d2c39

valeriupredoi approved these changes Feb 28, 2023

View reviewed changes

schlunma added 2 commits February 28, 2023 18:02

Revert "Avoid repeating deprecation warnings"

f0fbb07

This reverts commit 5eeed56.

Added missing full stop

2acbf4b

remi-kazeroni merged commit 40e00e8 into main Mar 1, 2023

remi-kazeroni deleted the search_esgf_option branch March 1, 2023 15:46

remi-kazeroni mentioned this pull request Mar 3, 2023

Draft changelog for v2.8.0 #1951

Closed

4 tasks

SarahAlidoost mentioned this pull request Apr 12, 2023

Checklist for updating lessons according to esmvaltool/esmvalcore 2.8 ESMValGroup/ESMValTool_Tutorial#278

Closed

7 tasks

rswamina mentioned this pull request May 1, 2023

Checklist for updating basic tutorial lessons for v2.8 ESMValGroup/ESMValTool_Tutorial#280

Closed

5 tasks

schlunma mentioned this pull request Oct 2, 2023

Removed deprecated configuration option offline #2213

Merged

9 tasks

schlunma mentioned this pull request Dec 18, 2023

Also read default custom CMOR tables if custom location is specified #2279

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combined `offline` and `always_search_esgf` into a single option `search_esgf` #1935

Combined `offline` and `always_search_esgf` into a single option `search_esgf` #1935

schlunma commented Feb 17, 2023 •

edited

Loading

schlunma commented Feb 28, 2023 •

edited

Loading

schlunma commented Feb 28, 2023 •

edited by bouweandela

Loading

bouweandela commented Feb 28, 2023

bouweandela commented Feb 28, 2023

schlunma commented Feb 28, 2023

bouweandela commented Feb 28, 2023

valeriupredoi left a comment

valeriupredoi left a comment

valeriupredoi left a comment

bouweandela commented Feb 28, 2023

schlunma commented Feb 28, 2023

valeriupredoi commented Feb 28, 2023

schlunma commented Feb 28, 2023

remi-kazeroni commented Mar 1, 2023

valeriupredoi commented Mar 1, 2023

Combined offline and always_search_esgf into a single option search_esgf #1935

Combined offline and always_search_esgf into a single option search_esgf #1935

Conversation

schlunma commented Feb 17, 2023 • edited Loading

Description

Deprecation

Before you get started

Checklist

schlunma commented Feb 28, 2023 • edited Loading

schlunma commented Feb 28, 2023 • edited by bouweandela Loading

bouweandela commented Feb 28, 2023

bouweandela commented Feb 28, 2023

schlunma commented Feb 28, 2023

bouweandela commented Feb 28, 2023

valeriupredoi left a comment

Choose a reason for hiding this comment

valeriupredoi left a comment

Choose a reason for hiding this comment

valeriupredoi left a comment

Choose a reason for hiding this comment

bouweandela commented Feb 28, 2023

schlunma commented Feb 28, 2023

valeriupredoi commented Feb 28, 2023

schlunma commented Feb 28, 2023

remi-kazeroni commented Mar 1, 2023

valeriupredoi commented Mar 1, 2023

Combined `offline` and `always_search_esgf` into a single option `search_esgf` #1935

Combined `offline` and `always_search_esgf` into a single option `search_esgf` #1935

schlunma commented Feb 17, 2023 •

edited

Loading

schlunma commented Feb 28, 2023 •

edited

Loading

schlunma commented Feb 28, 2023 •

edited by bouweandela

Loading