Skip to content

Commit

Permalink
Add depreciation warning in 1.0 for renaming package via cmd. (#1264)
Browse files Browse the repository at this point in the history
* 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
#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
  • Loading branch information
shagunsodhani authored Jan 7, 2021
1 parent 9331ca7 commit 8bea143
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 33 deletions.
15 changes: 15 additions & 0 deletions hydra/core/override_parser/types.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions news/1140.api_change
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate support for renaming packages via the command line.
8 changes: 4 additions & 4 deletions tests/test_config_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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})
Expand Down
27 changes: 0 additions & 27 deletions tests/test_examples/test_advanced_package_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"},
}
4 changes: 2 additions & 2 deletions tests/test_overrides_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 8bea143

Please sign in to comment.