Skip to content

Commit

Permalink
changed default for ConfigStore.store(package) to _group_
Browse files Browse the repository at this point in the history
  • Loading branch information
omry committed May 13, 2020
1 parent cb13cfa commit 167db11
Show file tree
Hide file tree
Showing 23 changed files with 58 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions examples/structured_configs_tutorial/4_defaults/my_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
# @package _group_
defaults:
- database: mysql
- db: mysql

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# @package _group_
user: omry
password: secret
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# @package _group_
user: postgre_user
password: drowssap
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)


Expand Down
6 changes: 1 addition & 5 deletions hydra/_internal/core_plugins/basic_sweeper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)


Expand Down
19 changes: 5 additions & 14 deletions hydra/core/config_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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":
Expand All @@ -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:
"""
Expand All @@ -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("/"):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class JobLibLauncherConf(ObjectConf):
ConfigStore.instance().store(
group="hydra/launcher",
name="joblib",
package="hydra.launcher",
node=JobLibLauncherConf,
provider="joblib_launcher",
)
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,6 @@ class NevergradSweeperConf(ObjectConf):
ConfigStore.instance().store(
group="hydra/sweeper",
name="nevergrad",
package="hydra.sweeper",
node=NevergradSweeperConf,
provider="nevergrad",
)
13 changes: 7 additions & 6 deletions tests/test_apps/config_source_test/structured.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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_")
Expand Down
16 changes: 4 additions & 12 deletions tests/test_config_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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=[])
Expand All @@ -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):
Expand Down
1 change: 1 addition & 0 deletions tests/test_hydra.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
9 changes: 3 additions & 6 deletions tests/test_structured_configs_tutorial.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions website/docs/tutorials/structured_config/10_config_store.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ 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:
"""
Stores a config node into the repository
: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.
"""
...
Expand Down
9 changes: 5 additions & 4 deletions website/docs/tutorials/structured_config/2_nesting_configs.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,19 @@ 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
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:
Expand Down
16 changes: 8 additions & 8 deletions website/docs/tutorials/structured_config/3_config_groups.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -34,16 +34,16 @@ 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:
print(cfg.pretty())
```
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
Expand Down Expand Up @@ -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}")
Expand All @@ -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)
```
4 changes: 2 additions & 2 deletions website/docs/tutorials/structured_config/4_defaults.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
6 changes: 3 additions & 3 deletions website/docs/tutorials/structured_config/5_schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 167db11

Please sign in to comment.