From 8bea143229219347291ed4022e01af2954664814 Mon Sep 17 00:00:00 2001 From: Shagun Sodhani <1321193+shagunsodhani@users.noreply.github.com> Date: Thu, 7 Jan 2021 07:02:09 -0500 Subject: [PATCH] Add depreciation warning in 1.0 for renaming package via cmd. (#1264) * Add deperication warning for overriding package via cmd. Support for overriding the package via the command line is deprecated since Hydra 1.0.5 and will be removed in Hydra 1.1. For more details, refer https://github.com/facebookresearch/hydra/issues/1140. * Correctly raise the warning after creating the override class * Update the tests to work with the depreciation warning * Fix formatting of the error message * Fix some broken tests * Simplify testing logic * Simplify tests * Simplify tests * Add two tests checking for package rename * Format message using dedent * Format message using dedent * Add NEWS Entry --- hydra/core/override_parser/types.py | 15 +++++++++++ news/1140.api_change | 1 + tests/test_config_loader.py | 8 +++--- .../test_advanced_package_overrides.py | 27 ------------------- tests/test_overrides_parser.py | 4 +-- 5 files changed, 22 insertions(+), 33 deletions(-) create mode 100644 news/1140.api_change diff --git a/hydra/core/override_parser/types.py b/hydra/core/override_parser/types.py index c3da476c5d0..7b835e9b1d9 100644 --- a/hydra/core/override_parser/types.py +++ b/hydra/core/override_parser/types.py @@ -1,10 +1,12 @@ # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved import decimal import fnmatch +import warnings from copy import copy from dataclasses import dataclass, field from enum import Enum from random import shuffle +from textwrap import dedent from typing import Any, Callable, Dict, Iterator, List, Optional, Set, Union, cast from omegaconf import OmegaConf @@ -235,6 +237,19 @@ class Override: # Configs repo config_loader: Optional[ConfigLoader] = None + def __post_init__(self) -> None: + if self.pkg2 is not None: + # DEPRECATED: remove in 1.1 + + msg = dedent( + """\n + Support for renaming packages via the command line is deprecated since Hydra 1.0.5. + The support will be removed in Hydra 1.1. + For more details, see https://github.com/facebookresearch/hydra/issues/1140. + """ + ) + warnings.warn(message=msg, category=UserWarning) + def is_delete(self) -> bool: """ :return: True if this override represents a deletion of a config value or config group option diff --git a/news/1140.api_change b/news/1140.api_change new file mode 100644 index 00000000000..3ff0aa22e14 --- /dev/null +++ b/news/1140.api_change @@ -0,0 +1 @@ +Deprecate support for renaming packages via the command line. \ No newline at end of file diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py index 7f1d0bea3dd..a32e8e76610 100644 --- a/tests/test_config_loader.py +++ b/tests/test_config_loader.py @@ -122,7 +122,7 @@ def test_load_with_optional_default(self, path: str) -> None: ], ) def test_load_changing_group_in_default( - self, path: str, override: str, expected: Dict[Any, Any] + self, path: str, override: str, expected: Dict[Any, Any], recwarn: Any ) -> None: config_loader = ConfigLoaderImpl( config_search_path=create_config_search_path(path) @@ -158,7 +158,7 @@ def test_load_changing_group_in_default( ], ) def test_load_changing_group_and_package_in_default( - self, path: str, overrides: List[str], expected: Any + self, path: str, overrides: List[str], expected: Any, recwarn: Any ) -> None: config_loader = ConfigLoaderImpl( config_search_path=create_config_search_path(f"{path}/package_tests") @@ -205,7 +205,7 @@ def test_load_changing_group_and_package_in_default( ], ) def test_override_compose_two_package_one_group( - self, path: str, overrides: List[str], expected: Any + self, path: str, overrides: List[str], expected: Any, recwarn: Any ) -> None: config_loader = ConfigLoaderImpl( config_search_path=create_config_search_path(f"{path}/package_tests") @@ -1042,7 +1042,7 @@ def test_complex_defaults(overrides: Any, expected: Any) -> None: ], ) def test_apply_overrides_to_defaults( - input_defaults: List[str], overrides: List[str], expected: Any + input_defaults: List[str], overrides: List[str], expected: Any, recwarn: Any ) -> None: defaults = ConfigLoaderImpl._parse_defaults( OmegaConf.create({"defaults": input_defaults}) diff --git a/tests/test_examples/test_advanced_package_overrides.py b/tests/test_examples/test_advanced_package_overrides.py index d1e3783bd7a..a59b9d6c29c 100644 --- a/tests/test_examples/test_advanced_package_overrides.py +++ b/tests/test_examples/test_advanced_package_overrides.py @@ -19,20 +19,6 @@ def test_advanced_package_override_simple(tmpdir: Path) -> None: } -def test_advanced_package_override_simple_with_cli_pakcage_override( - tmpdir: Path, -) -> None: - cmd = [ - "examples/advanced/package_overrides/simple.py", - "hydra.run.dir=" + str(tmpdir), - "db@:source=mysql", - ] - result, _err = get_run_output(cmd) - assert OmegaConf.create(result) == { - "source": {"driver": "mysql", "user": "omry", "pass": "secret"} - } - - def test_advanced_package_override_two_packages(tmpdir: Path) -> None: cmd = [ "examples/advanced/package_overrides/two_packages.py", @@ -43,16 +29,3 @@ def test_advanced_package_override_two_packages(tmpdir: Path) -> None: "source": {"driver": "mysql", "user": "omry", "pass": "secret"}, "destination": {"driver": "mysql", "user": "omry", "pass": "secret"}, } - - -def test_advanced_package_override_two_packages_with_cli_override(tmpdir: Path) -> None: - cmd = [ - "examples/advanced/package_overrides/two_packages.py", - "hydra.run.dir=" + str(tmpdir), - "db@destination:backup=mysql", - ] - result, _err = get_run_output(cmd) - assert OmegaConf.create(result) == { - "source": {"driver": "mysql", "user": "omry", "pass": "secret"}, - "backup": {"driver": "mysql", "user": "omry", "pass": "secret"}, - } diff --git a/tests/test_overrides_parser.py b/tests/test_overrides_parser.py index 5e7af0e2572..eebe58d8bac 100644 --- a/tests/test_overrides_parser.py +++ b/tests/test_overrides_parser.py @@ -658,7 +658,7 @@ def test_primitive(value: str, expected: Any) -> None: pytest.param("abc@:pkg2=xyz", True, id="rename_from_current"), ], ) -def test_key_rename(value: str, expected: bool) -> None: +def test_key_rename(value: str, expected: bool, recwarn: Any) -> None: ret = parse_rule(value, "override") assert ret.is_package_rename() == expected @@ -917,7 +917,7 @@ def test_parse_overrides() -> None: pytest.param("~key@:pkg2=value", "~key@:pkg2", id="~key@:pkg2"), ], ) -def test_get_key_element(override: str, expected: str) -> None: +def test_get_key_element(override: str, expected: str, recwarn: Any) -> None: ret = parse_rule(override, "override") assert ret.get_key_element() == expected