Skip to content

Commit

Permalink
Remove useless deprecation warnings
Browse files Browse the repository at this point in the history
Why:

There was still access to deprecated options in the `core` which were
triggering deprecation warnings event though the user was not using
any of these.

How:

Adjust access to config to use the proper configuration values
and update yaml reading to transfer deprecated settings to proper ones
with warning for users.
  • Loading branch information
bouthilx committed Apr 2, 2020
1 parent 9dbfdef commit 400f86a
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/orion/core/evc/conflicts.py
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ def get_nameless_args(cls, config, user_script_config=None,
return ""

if user_script_config is None:
user_script_config = orion.core.config.user_script_config
user_script_config = orion.core.config.worker.user_script_config
if non_monitored_arguments is None:
non_monitored_arguments = orion.core.config.evc.non_monitored_arguments

Expand Down Expand Up @@ -1387,7 +1387,7 @@ def get_nameless_config(cls, config, user_script_config=None, **branching_kwargs
return ""

if user_script_config is None:
user_script_config = orion.core.config.user_script_config
user_script_config = orion.core.config.worker.user_script_config

parser = OrionCmdlineParser(user_script_config)
parser.set_state_dict(config['metadata']['parser'])
Expand Down
50 changes: 43 additions & 7 deletions src/orion/core/io/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
Highly inspired from https://github.com/mila-iqia/blocks/blob/master/blocks/config.py.
"""
import contextlib
import logging
import os

Expand All @@ -26,6 +27,16 @@
NOT_SET = object()


@contextlib.contextmanager
def _disable_logger(disable=True):
if disable:
logger.disabled = True
yield

if disable:
logger.disabled = False


class ConfigurationError(Exception):
"""Error raised when a configuration value is requested but not set."""

Expand Down Expand Up @@ -61,7 +72,8 @@ class Configuration:
"""

SPECIAL_KEYS = ['_config', '_subconfigs', '_yaml', '_default', '_env_var', '_help']
SPECIAL_KEYS = ['_config', '_subconfigs', '_yaml', '_default', '_env_var', '_help',
'_deprecated']

def __init__(self):
self._config = {}
Expand All @@ -87,9 +99,13 @@ def load_yaml(self, path):
return
# implies that yaml must be in dict form
for key, value in flatten(cfg).items():
default = self[key]
default = self[key + '._default']
deprecated = self[key + '._deprecated']
logger.debug('Overwritting "%s" default %s with %s', key, default, value)
self[key + '._yaml'] = value
if deprecated and deprecated.get('alternative'):
logger.debug('Overwritting "%s" default %s with %s', key, default, value)
self[deprecated.get('alternative') + '._yaml'] = value

def __getattr__(self, key):
"""Get the value of the option
Expand Down Expand Up @@ -189,8 +205,26 @@ def _deprecate(self, key, value=NOT_SET):
if 'alternative' in deprecate:
message += " Use `%s` instead."
args.append(deprecate['alternative'])

logger.warning(message, *args)

def get(self, key, deprecated='warn'):
"""Access value
Parameters
----------
key: str
Key to access in the configuration. Similar to config.key.
deprecated: str, optional
If 'warn', the access to deprecated options will log a deprecation warning.
else if 'ignore', no warning will be logged for access to deprecated options.
"""
with _disable_logger(disable=(deprecated == 'ignore')):
value = self[key]

return value

def _validate(self, key, value):
"""Validate the (key, value) option
Expand Down Expand Up @@ -263,7 +297,7 @@ def __getitem__(self, key):

# Recursively in sub configurations
if len(keys) == 2 and keys[1] in self.SPECIAL_KEYS:
return self._config[keys[0]][keys[1][1:]]
return self._config[keys[0]].get(keys[1][1:], None)
elif len(keys) > 1:
subconfig = getattr(self, keys[0])
if subconfig is None:
Expand Down Expand Up @@ -371,10 +405,12 @@ def __contains__(self, key):
def to_dict(self):
"""Return a dictionary representation of the configuration"""
config = dict()
for key in self._config:
config[key] = self[key]

for key in self._subconfigs:
config[key] = self[key].to_dict()
with _disable_logger():
for key in self._config:
config[key] = self[key]

for key in self._subconfigs:
config[key] = self[key].to_dict()

return config
5 changes: 4 additions & 1 deletion src/orion/core/io/experiment_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ def create_experiment(name, version, space, **kwargs):
"""
experiment = Experiment(name=name, version=version)
experiment._id = kwargs.get('_id', None) # pylint:disable=protected-access
experiment.pool_size = kwargs.get('pool_size', orion.core.config.experiment.pool_size)
experiment.pool_size = kwargs.get('pool_size')
if experiment.pool_size is None:
experiment.pool_size = orion.core.config.experiment.get(
'pool_size', deprecated='ignore')
experiment.max_trials = kwargs.get('max_trials', orion.core.config.experiment.max_trials)
experiment.space = _instantiate_space(space)
experiment.algorithms = _instantiate_algo(experiment.space, kwargs.get('algorithms'))
Expand Down
2 changes: 1 addition & 1 deletion src/orion/core/io/resolve_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ def fetch_metadata(user=None, user_args=None):
if len(user_args) == 1 and user_args[0] == '':
user_args = []

cmdline_parser = OrionCmdlineParser(config.user_script_config)
cmdline_parser = OrionCmdlineParser(config.worker.user_script_config)
cmdline_parser.parse(user_args)

if cmdline_parser.user_script:
Expand Down
2 changes: 1 addition & 1 deletion src/orion/core/utils/backward.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def populate_priors(metadata):

update_user_args(metadata)

parser = OrionCmdlineParser(orion.core.config.user_script_config,
parser = OrionCmdlineParser(orion.core.config.worker.user_script_config,
allow_non_existing_user_script=True)
parser.parse(metadata["user_args"])
metadata["parser"] = parser.get_state_dict()
Expand Down
2 changes: 1 addition & 1 deletion src/orion/core/worker/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def __init__(self, experiment, heartbeat=None, user_script_config=None,
heartbeat = orion.core.config.worker.heartbeat

if user_script_config is None:
user_script_config = orion.core.config.user_script_config
user_script_config = orion.core.config.worker.user_script_config

if interrupt_signal_code is None:
interrupt_signal_code = orion.core.config.worker.interrupt_signal_code
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/demo/test_demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ def test_demo_with_shutdown_quickly(monkeypatch):
def test_demo_with_nondefault_config_keyword(database, monkeypatch):
"""Check that the user script configuration file is correctly used with a new keyword."""
monkeypatch.chdir(os.path.dirname(os.path.abspath(__file__)))
orion.core.config.user_script_config = 'configuration'
orion.core.config.worker.user_script_config = 'configuration'
orion.core.cli.main(["hunt", "--config", "./orion_config_other.yaml",
"./black_box_w_config_other.py", "--configuration", "script_config.yaml"])

Expand Down Expand Up @@ -579,7 +579,7 @@ def test_demo_with_nondefault_config_keyword(database, monkeypatch):
assert params[0]['type'] == 'real'
assert (params[0]['value'] - 34.56789) < 1e-5

orion.core.config.user_script_config = 'config'
orion.core.config.worker.user_script_config = 'config'


@pytest.mark.usefixtures("clean_db")
Expand Down
30 changes: 30 additions & 0 deletions tests/unittests/core/io/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,3 +492,33 @@ def test_deprecate_option_print_with_different_name(caplog):
assert caplog.record_tuples == [(
'orion.core.io.config', logging.WARNING,
'(DEPRECATED) Option `nested.option` will be removed in v1.0. Use `None! T_T` instead.')]


def test_get_deprecated_key(caplog):
"""Verify deprecation warning using get()."""
config = Configuration()
config.add_option(
'option', option_type=str, default='hello',
deprecate=dict(version='v1.0', alternative='None! T_T'))

# Access the deprecated option and trigger a warning.
with caplog.at_level(logging.WARNING, logger="orion.core.io.config"):
assert config.get('option') == 'hello'

assert caplog.record_tuples == [(
'orion.core.io.config', logging.WARNING,
'(DEPRECATED) Option `option` will be removed in v1.0. Use `None! T_T` instead.')]


def test_get_deprecated_key_ignore_warning(caplog):
"""Verify deprecation warning using get(deprecated='ignore')."""
config = Configuration()
config.add_option(
'option', option_type=str, default='hello',
deprecate=dict(version='v1.0', alternative='None! T_T'))

# Access the deprecated option and trigger a warning.
with caplog.at_level(logging.WARNING, logger="orion.core.io.config"):
assert config.get('option', deprecated='ignore') == 'hello'

assert caplog.record_tuples == []

0 comments on commit 400f86a

Please sign in to comment.