From 167db11e3c42e218ff3028d463e2b0b15f5677f7 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Wed, 13 May 2020 10:16:01 -0700 Subject: [PATCH] changed default for ConfigStore.store(package) to _group_ --- ...h_node_path.py => nesting_with_package.py} | 0 .../3_config_groups/my_app.py | 4 ++-- .../my_app_with_inheritance.py | 4 ++-- .../4_defaults/my_app.py | 4 ++-- .../conf/config.yaml | 3 ++- .../conf/database/mysql.yaml | 3 --- .../conf/database/postgresql.yaml | 3 --- .../conf/db/mysql.yaml | 3 +++ .../conf/db/postgresql.yaml | 3 +++ .../5_structured_config_schema/my_app.py | 10 ++-------- hydra/_internal/core_plugins/basic_sweeper.py | 6 +----- hydra/core/config_store.py | 19 +++++-------------- .../hydra_joblib_launcher/config.py | 1 - .../hydra_nevergrad_sweeper/config.py | 2 +- .../config_source_test/structured.py | 13 +++++++------ tests/test_config_loader.py | 16 ++++------------ tests/test_hydra.py | 1 + tests/test_structured_configs_tutorial.py | 9 +++------ .../structured_config/10_config_store.md | 4 ++-- .../structured_config/2_nesting_configs.md | 9 +++++---- .../structured_config/3_config_groups.md | 16 ++++++++-------- .../tutorials/structured_config/4_defaults.md | 4 ++-- .../tutorials/structured_config/5_schema.md | 6 +++--- 23 files changed, 58 insertions(+), 85 deletions(-) rename examples/structured_configs_tutorial/2_nesting_configs/{nesting_with_node_path.py => nesting_with_package.py} (100%) delete mode 100644 examples/structured_configs_tutorial/5_structured_config_schema/conf/database/mysql.yaml delete mode 100644 examples/structured_configs_tutorial/5_structured_config_schema/conf/database/postgresql.yaml create mode 100644 examples/structured_configs_tutorial/5_structured_config_schema/conf/db/mysql.yaml create mode 100644 examples/structured_configs_tutorial/5_structured_config_schema/conf/db/postgresql.yaml diff --git a/examples/structured_configs_tutorial/2_nesting_configs/nesting_with_node_path.py b/examples/structured_configs_tutorial/2_nesting_configs/nesting_with_package.py similarity index 100% rename from examples/structured_configs_tutorial/2_nesting_configs/nesting_with_node_path.py rename to examples/structured_configs_tutorial/2_nesting_configs/nesting_with_package.py diff --git a/examples/structured_configs_tutorial/3_config_groups/my_app.py b/examples/structured_configs_tutorial/3_config_groups/my_app.py index f1862108840..ad961efd89a 100644 --- a/examples/structured_configs_tutorial/3_config_groups/my_app.py +++ b/examples/structured_configs_tutorial/3_config_groups/my_app.py @@ -31,8 +31,8 @@ class Config(DictConfig): cs = ConfigStore.instance() cs.store(name="config", node=Config) -cs.store(group="database", name="mysql", package="db", node=MySQLConfig) -cs.store(group="database", name="postgresql", package="db", node=PostGreSQLConfig) +cs.store(group="db", name="mysql", node=MySQLConfig) +cs.store(group="db", name="postgresql", node=PostGreSQLConfig) @hydra.main(config_name="config") diff --git a/examples/structured_configs_tutorial/3_config_groups/my_app_with_inheritance.py b/examples/structured_configs_tutorial/3_config_groups/my_app_with_inheritance.py index 692bb346215..9d5f51eb7d0 100644 --- a/examples/structured_configs_tutorial/3_config_groups/my_app_with_inheritance.py +++ b/examples/structured_configs_tutorial/3_config_groups/my_app_with_inheritance.py @@ -35,8 +35,8 @@ class Config(DictConfig): cs = ConfigStore.instance() cs.store(name="config", node=Config) -cs.store(group="database", name="mysql", package="db", node=MySQLConfig) -cs.store(group="database", name="postgresql", package="db", node=PostGreSQLConfig) +cs.store(group="db", name="mysql", node=MySQLConfig) +cs.store(group="db", name="postgresql", node=PostGreSQLConfig) def connect_mysql(cfg: MySQLConfig) -> None: diff --git a/examples/structured_configs_tutorial/4_defaults/my_app.py b/examples/structured_configs_tutorial/4_defaults/my_app.py index 810bd001111..4e739b5c59e 100644 --- a/examples/structured_configs_tutorial/4_defaults/my_app.py +++ b/examples/structured_configs_tutorial/4_defaults/my_app.py @@ -41,8 +41,8 @@ class Config(DictConfig): cs = ConfigStore.instance() -cs.store(group="db", name="mysql", package="db", node=MySQLConfig) -cs.store(group="db", name="postgresql", package="db", node=PostGreSQLConfig) +cs.store(group="db", name="mysql", node=MySQLConfig) +cs.store(group="db", name="postgresql", node=PostGreSQLConfig) cs.store(name="config", node=Config) diff --git a/examples/structured_configs_tutorial/5_structured_config_schema/conf/config.yaml b/examples/structured_configs_tutorial/5_structured_config_schema/conf/config.yaml index 33cdc3cf05c..23f8e63ce93 100644 --- a/examples/structured_configs_tutorial/5_structured_config_schema/conf/config.yaml +++ b/examples/structured_configs_tutorial/5_structured_config_schema/conf/config.yaml @@ -1,2 +1,3 @@ +# @package _group_ defaults: - - database: mysql \ No newline at end of file + - db: mysql \ No newline at end of file diff --git a/examples/structured_configs_tutorial/5_structured_config_schema/conf/database/mysql.yaml b/examples/structured_configs_tutorial/5_structured_config_schema/conf/database/mysql.yaml deleted file mode 100644 index 57023c18c11..00000000000 --- a/examples/structured_configs_tutorial/5_structured_config_schema/conf/database/mysql.yaml +++ /dev/null @@ -1,3 +0,0 @@ -db: - user: omry - password: secret diff --git a/examples/structured_configs_tutorial/5_structured_config_schema/conf/database/postgresql.yaml b/examples/structured_configs_tutorial/5_structured_config_schema/conf/database/postgresql.yaml deleted file mode 100644 index c961a490b25..00000000000 --- a/examples/structured_configs_tutorial/5_structured_config_schema/conf/database/postgresql.yaml +++ /dev/null @@ -1,3 +0,0 @@ -db: - user: postgre_user - password: drowssap diff --git a/examples/structured_configs_tutorial/5_structured_config_schema/conf/db/mysql.yaml b/examples/structured_configs_tutorial/5_structured_config_schema/conf/db/mysql.yaml new file mode 100644 index 00000000000..f3f04415350 --- /dev/null +++ b/examples/structured_configs_tutorial/5_structured_config_schema/conf/db/mysql.yaml @@ -0,0 +1,3 @@ +# @package _group_ +user: omry +password: secret diff --git a/examples/structured_configs_tutorial/5_structured_config_schema/conf/db/postgresql.yaml b/examples/structured_configs_tutorial/5_structured_config_schema/conf/db/postgresql.yaml new file mode 100644 index 00000000000..0b17d1889d1 --- /dev/null +++ b/examples/structured_configs_tutorial/5_structured_config_schema/conf/db/postgresql.yaml @@ -0,0 +1,3 @@ +# @package _group_ +user: postgre_user +password: drowssap diff --git a/examples/structured_configs_tutorial/5_structured_config_schema/my_app.py b/examples/structured_configs_tutorial/5_structured_config_schema/my_app.py index 06c7147f590..6a21e6538c0 100644 --- a/examples/structured_configs_tutorial/5_structured_config_schema/my_app.py +++ b/examples/structured_configs_tutorial/5_structured_config_schema/my_app.py @@ -33,15 +33,9 @@ class PostGreSQLConfig(DBConfig): # registering db/mysql and db/postgresql schemas. cs = ConfigStore.instance() +cs.store(group="db", name="mysql", node=MySQLConfig, provider="main") cs.store( - group="database", name="mysql", package="db", node=MySQLConfig, provider="main" -) -cs.store( - group="database", - name="postgresql", - package="db", - node=PostGreSQLConfig, - provider="main", + group="db", name="postgresql", node=PostGreSQLConfig, provider="main", ) diff --git a/hydra/_internal/core_plugins/basic_sweeper.py b/hydra/_internal/core_plugins/basic_sweeper.py index 0261f954547..2fb28e66e30 100644 --- a/hydra/_internal/core_plugins/basic_sweeper.py +++ b/hydra/_internal/core_plugins/basic_sweeper.py @@ -39,11 +39,7 @@ class Params: ConfigStore.instance().store( - group="hydra/sweeper", - name="basic", - node=BasicSweeperConf, - package="hydra.sweeper", - provider="hydra", + group="hydra/sweeper", name="basic", node=BasicSweeperConf, provider="hydra", ) diff --git a/hydra/core/config_store.py b/hydra/core/config_store.py index bc30a40db4f..e68237faa89 100644 --- a/hydra/core/config_store.py +++ b/hydra/core/config_store.py @@ -3,7 +3,7 @@ from dataclasses import dataclass from typing import Any, Dict, List, Optional -from omegaconf import OmegaConf +from omegaconf import DictConfig, OmegaConf from hydra.core.object_type import ObjectType from hydra.core.singleton import Singleton @@ -35,15 +35,12 @@ def __exit__(self, exc_type: Any, exc_value: Any, exc_traceback: Any) -> Any: @dataclass class ConfigNode: name: str - node: Any # TODO: DictConfig or Container? + node: DictConfig group: Optional[str] package: Optional[str] provider: Optional[str] -NO_DEFAULT_PACKAGE = str("_NO_DEFAULT_PACKAGE_") - - class ConfigStore(metaclass=Singleton): @staticmethod def instance(*args: Any, **kwargs: Any) -> "ConfigStore": @@ -56,10 +53,10 @@ def __init__(self) -> None: def store( self, - name: str, + name: str, # TODO: config_name node: Any, - group: Optional[str] = None, - package: Optional[str] = NO_DEFAULT_PACKAGE, # TODO: use _group_ by default. + group: Optional[str] = None, # TODO: config group + package: Optional[str] = "_group_", provider: Optional[str] = None, ) -> None: """ @@ -71,12 +68,6 @@ def store( :param provider: the name of the module/app providing this config. Helps debugging. """ - if package == NO_DEFAULT_PACKAGE: - package = "_global_" - # TODO: warn the user if we are defaulting to _global_ and they should make - # an explicit selection recommended _group_. - # OR just change the default to _group_ (probably better) - cur = self.repo if group is not None: for d in group.split("/"): diff --git a/plugins/hydra_joblib_launcher/hydra_plugins/hydra_joblib_launcher/config.py b/plugins/hydra_joblib_launcher/hydra_plugins/hydra_joblib_launcher/config.py index 254392652e7..5a9dbda5d2a 100644 --- a/plugins/hydra_joblib_launcher/hydra_plugins/hydra_joblib_launcher/config.py +++ b/plugins/hydra_joblib_launcher/hydra_plugins/hydra_joblib_launcher/config.py @@ -51,7 +51,6 @@ class JobLibLauncherConf(ObjectConf): ConfigStore.instance().store( group="hydra/launcher", name="joblib", - package="hydra.launcher", node=JobLibLauncherConf, provider="joblib_launcher", ) diff --git a/plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/config.py b/plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/config.py index 53ab5c3c881..14f78a7bc03 100644 --- a/plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/config.py +++ b/plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/config.py @@ -91,6 +91,6 @@ class NevergradSweeperConf(ObjectConf): ConfigStore.instance().store( group="hydra/sweeper", name="nevergrad", - package="hydra.sweeper", node=NevergradSweeperConf, + provider="nevergrad", ) diff --git a/tests/test_apps/config_source_test/structured.py b/tests/test_apps/config_source_test/structured.py index 253e15171d0..5b2aeb7e209 100644 --- a/tests/test_apps/config_source_test/structured.py +++ b/tests/test_apps/config_source_test/structured.py @@ -50,10 +50,11 @@ class Optimizer: s = ConfigStore.instance() s.store(name="config_without_group", node=ConfigWithoutGroup) s.store(name="dataset", node={"dataset_yaml": True}) -s.store(group="dataset", name="cifar10", node=Cifar10, package="dataset") -s.store(group="dataset", name="imagenet.yaml", node=ImageNet, package="dataset") -s.store(group="optimizer", name="adam", node=Adam, package="optimizer") -s.store(group="optimizer", name="nesterov", node=Nesterov, package="optimizer") +s.store(group="dataset", name="cifar10", node=Cifar10) +s.store(group="dataset", name="imagenet.yaml", node=ImageNet) +s.store(group="optimizer", name="adam", node=Adam) +s.store(group="optimizer", name="nesterov", node=Nesterov) +# TODO : package here is wrong, why is it passing the tests? s.store( group="level1/level2", name="nested1", node={"l1_l2_n1": True}, package="optimizer" ) @@ -62,8 +63,8 @@ class Optimizer: group="level1/level2", name="nested2", node={"l1_l2_n2": True}, package="optimizer" ) - -s.store(group="package_test", name="none", node={"foo": "bar"}) +# TODO: remove _global_ once default for config file based package is changed to _group_ +s.store(group="package_test", name="none", node={"foo": "bar"}, package="_global_") s.store(group="package_test", name="explicit", node={"foo": "bar"}, package="a.b") s.store(group="package_test", name="global", node={"foo": "bar"}, package="_global_") s.store(group="package_test", name="group", node={"foo": "bar"}, package="_group_") diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py index aaf499636a0..c1500836d92 100644 --- a/tests/test_config_loader.py +++ b/tests/test_config_loader.py @@ -222,11 +222,7 @@ def test_compose_file_with_dot(self, path: str) -> None: def test_load_config_with_schema(self, restore_singletons: Any, path: str) -> None: ConfigStore.instance().store( - group="db", - name="mysql", - node=MySQLConfig, - package="db", - provider="test_provider", + group="db", name="mysql", node=MySQLConfig, provider="test_provider", ) config_loader = ConfigLoaderImpl( @@ -503,11 +499,7 @@ def test_load_schema_as_config(restore_singletons: Any) -> None: Load structured config as a configuration """ ConfigStore.instance().store( - group="db", - name="mysql", - node=MySQLConfig, - package="db", - provider="test_provider", + group="db", name="mysql", node=MySQLConfig, provider="test_provider", ) config_loader = ConfigLoaderImpl(config_search_path=create_config_search_path(None)) @@ -561,7 +553,7 @@ def test_overlapping_schemas(restore_singletons: Any) -> None: cs = ConfigStore.instance() cs.store(name="config", node=Config) - cs.store(group="plugin", name="concrete", node=ConcretePlugin, package="plugin") + cs.store(group="plugin", name="concrete", node=ConcretePlugin) config_loader = ConfigLoaderImpl(config_search_path=create_config_search_path(None)) cfg = config_loader.load_configuration(config_name="config", overrides=[]) @@ -587,7 +579,7 @@ def test_overlapping_schemas(restore_singletons: Any) -> None: def test_invalid_plugin_merge(restore_singletons: Any) -> Any: cs = ConfigStore.instance() cs.store(name="config", node=Config) - cs.store(group="plugin", name="invalid", node=InvalidPlugin, package="plugin") + cs.store(group="plugin", name="invalid", node=InvalidPlugin) cl = ConfigLoaderImpl(config_search_path=create_config_search_path(None)) with pytest.raises(HydraException): diff --git a/tests/test_hydra.py b/tests/test_hydra.py index 28ffcd9fa33..f30dcbe2ce1 100644 --- a/tests/test_hydra.py +++ b/tests/test_hydra.py @@ -1,5 +1,6 @@ # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved import os +import re import subprocess import sys from pathlib import Path diff --git a/tests/test_structured_configs_tutorial.py b/tests/test_structured_configs_tutorial.py index 8f2518854d1..c5a783be39a 100644 --- a/tests/test_structured_configs_tutorial.py +++ b/tests/test_structured_configs_tutorial.py @@ -88,7 +88,7 @@ def test_structured_configs_2_nesting_configs__with_dataclass(tmpdir: Path) -> N def test_structured_configs_2_nesting_configs__with_node_path(tmpdir: Path) -> None: cmd = [ sys.executable, - "examples/structured_configs_tutorial/2_nesting_configs/nesting_with_node_path.py", + "examples/structured_configs_tutorial/2_nesting_configs/nesting_with_package.py", "hydra.run.dir=" + str(tmpdir), ] result = check_output(cmd) @@ -109,10 +109,7 @@ def test_structured_configs_2_nesting_configs__with_ad_hoc_node(tmpdir: Path) -> "overrides,expected", [ ([], {"db": "???"}), - ( - ["database=mysql"], - {"db": {"driver": "mysql", "host": "localhost", "port": 3306}}, - ), + (["db=mysql"], {"db": {"driver": "mysql", "host": "localhost", "port": 3306}},), ], ) def test_structured_configs_3_config_groups( @@ -134,7 +131,7 @@ def test_structured_configs_3_config_groups_with_inheritance(tmpdir: Path) -> No sys.executable, "examples/structured_configs_tutorial/3_config_groups/my_app_with_inheritance.py", "hydra.run.dir=" + str(tmpdir), - "database=mysql", + "db=mysql", ] result = check_output(cmd) assert result.decode().rstrip() == "Connecting to MySQL: localhost:3306" diff --git a/website/docs/tutorials/structured_config/10_config_store.md b/website/docs/tutorials/structured_config/10_config_store.md index 858e2437ca0..de53a74096d 100644 --- a/website/docs/tutorials/structured_config/10_config_store.md +++ b/website/docs/tutorials/structured_config/10_config_store.md @@ -12,7 +12,7 @@ class ConfigStore(metaclass=Singleton): name: str, node: Any, group: Optional[str] = None, - path: Optional[str] = None, + package: Optional[str] = "_group_", provider: Optional[str] = None, ) -> None: """ @@ -20,7 +20,7 @@ class ConfigStore(metaclass=Singleton): :param name: config name :param node: config node, can be DictConfig, ListConfig, Structured configs and even dict and list :param group: config group, subgroup separator is '/', for example hydra/launcher - :param path: Config node parent hierarchy. child separator is '.', for example foo.bar.baz + :param package: Config node package :param provider: the name of the module/app providing this config. Helps debugging. """ ... diff --git a/website/docs/tutorials/structured_config/2_nesting_configs.md b/website/docs/tutorials/structured_config/2_nesting_configs.md index 6fc9221e071..7fe976b53da 100644 --- a/website/docs/tutorials/structured_config/2_nesting_configs.md +++ b/website/docs/tutorials/structured_config/2_nesting_configs.md @@ -34,10 +34,11 @@ if __name__ == "__main__": my_app() ``` -### Nesting by specifying a node path +### Nesting by specifying a package If for some reason you do not want to have a top level config class, you can still place MySQLConfig -in a specific path in the final configuration object. To do that, use the `path` parameter to specify the path. -You can use dot-notation to create multiple parent nodes as needed (E.G. `path="foo.bar.baz"`) +in a specific package in the final configuration object. To do that, use the `package` parameter to specify the package. +You can use dot-notation to create multiple parent nodes as needed (E.G. `path="foo.bar.baz"`). +By default, the package parameter is `_group_`. ```python @dataclass @@ -45,7 +46,7 @@ class MySQLConfig: ... cfg_store = ConfigStore.instance() -cfg_store.store(name="config", path="db", node=MySQLConfig) +cfg_store.store(name="config", node=MySQLConfig, package="db") @hydra.main(config_name="config") def my_app(cfg: DictConfig) -> None: diff --git a/website/docs/tutorials/structured_config/3_config_groups.md b/website/docs/tutorials/structured_config/3_config_groups.md index 732d4e229c4..73684a7a48b 100644 --- a/website/docs/tutorials/structured_config/3_config_groups.md +++ b/website/docs/tutorials/structured_config/3_config_groups.md @@ -3,8 +3,8 @@ id: config_groups title: Config groups --- -This example adds `mysql` and `postgresql` configs into the config group `database`. -The config group `database` corresponds to the directory name inside the config directory in config-file based examples. +This example adds `mysql` and `postgresql` configs into the config group `db`. +The config group `db` corresponds to the directory name inside the config directory in config-file based examples. Noteworthy things in the example: - The two config classes `MySQLConfig` and `PostGreSQLConfig` have no common superclass @@ -34,8 +34,8 @@ class Config(DictConfig): cs = ConfigStore.instance() cs.store(name="config", node=Config) -cs.store(group="database", name="mysql", path="db", node=MySQLConfig) -cs.store(group="database", name="postgresql", path="db", node=PostGreSQLConfig) +cs.store(group="db", name="mysql", node=MySQLConfig) +cs.store(group="db", name="postgresql", node=PostGreSQLConfig) @hydra.main(config_name="config") def my_app(cfg: Config) -> None: @@ -43,7 +43,7 @@ def my_app(cfg: Config) -> None: ``` You can select the database from the command line: ```yaml -$ python my_app.py database=postgresql +$ python my_app.py db=postgresql db: driver: postgresql host: localhost @@ -84,8 +84,8 @@ class Config(DictConfig): cs = ConfigStore.instance() cs.store(name="config", node=Config) -cs.store(group="database", name="mysql", path="db", node=MySQLConfig) -cs.store(group="database", name="postgresql", path="db", node=PostGreSQLConfig) +cs.store(group="db", name="mysql", node=MySQLConfig) +cs.store(group="db", name="postgresql", node=PostGreSQLConfig) def connect_mysql(cfg: MySQLConfig) -> None: print(f"Connecting to MySQL: {cfg.host}:{cfg.port}") @@ -107,6 +107,6 @@ def my_app(cfg: Config) -> None: Example output: ``` -$ python my_app_with_inheritance.py database=postgresql +$ python my_app_with_inheritance.py db=postgresql Connecting to PostGreSQL: localhost:5432 (timeout=10) ``` \ No newline at end of file diff --git a/website/docs/tutorials/structured_config/4_defaults.md b/website/docs/tutorials/structured_config/4_defaults.md index f37c63cfbb8..cda964568b1 100644 --- a/website/docs/tutorials/structured_config/4_defaults.md +++ b/website/docs/tutorials/structured_config/4_defaults.md @@ -30,8 +30,8 @@ class Config(DictConfig): cs = ConfigStore.instance() -cs.store(group="database", name="mysql", path="db", node=MySQLConfig) -cs.store(group="database", name="postgresql", path="db", node=PostGreSQLConfig) +cs.store(group="db", name="mysql", node=MySQLConfig) +cs.store(group="db", name="postgresql", node=PostGreSQLConfig) cs.store(name="config", node=Config) diff --git a/website/docs/tutorials/structured_config/5_schema.md b/website/docs/tutorials/structured_config/5_schema.md index 26110ddb3ae..3a873d9f8aa 100644 --- a/website/docs/tutorials/structured_config/5_schema.md +++ b/website/docs/tutorials/structured_config/5_schema.md @@ -18,7 +18,7 @@ conf/ └── postgresql.yaml ``` -The Structurd configs below are stored as db/mysql and db/postgresql. They will be used as schema +The Structurd Configs below are stored as db/mysql and db/postgresql. They will be used as schema when we load the corresponding config files. ```python @@ -48,8 +48,8 @@ class PostGreSQLConfig(DBConfig): # registering db/mysql and db/postgresql schemas. ss = ConfigStore.instance() -ss.store(group="db", name="mysql", path="db", node=MySQLConfig) -ss.store(group="db", name="postgresql", path="db", node=PostGreSQLConfig) +ss.store(group="db", name="mysql", node=MySQLConfig) +ss.store(group="db", name="postgresql", node=PostGreSQLConfig) # config here is config.yaml under the conf directory.