Skip to content

Commit

Permalink
Added a warning if primary config is missing _self_
Browse files Browse the repository at this point in the history
  • Loading branch information
omry committed Aug 9, 2021
1 parent bcf2c57 commit b4a5bc2
Show file tree
Hide file tree
Showing 31 changed files with 472 additions and 26 deletions.
1 change: 1 addition & 0 deletions examples/advanced/config_search_path/conf/config.yaml
@@ -1,5 +1,6 @@
defaults:
- dataset: cifar10
- _self_

hydra:
searchpath:
Expand Down
@@ -1,8 +1,11 @@
defaults:
- db: mysql
- _self_

app:
user: ${oc.env:USER}

num1: 10
num2: 20

db: ???
@@ -1,3 +1,2 @@
# @package db
host: localhost
port: 3306
7 changes: 6 additions & 1 deletion examples/instantiate/schema/my_app.py
Expand Up @@ -54,7 +54,12 @@ class PostGreSQLConfig(DBConfig):

@dataclass
class Config:
defaults: List[Any] = field(default_factory=lambda: [{"db": "mysql"}])
defaults: List[Any] = field(
default_factory=lambda: [
"_self_",
{"db": "mysql"},
]
)
db: DBConfig = MISSING


Expand Down
1 change: 1 addition & 0 deletions examples/instantiate/schema_recursive/config.yaml
Expand Up @@ -11,3 +11,4 @@ tree:

defaults:
- config_schema
- _self_
@@ -1,5 +1,7 @@
defaults:
- base_config
- db: mysql
# You typically want _self_ somewhere after the schema (base_config)
- _self_

debug: true
@@ -1,5 +1,7 @@
defaults:
- base_config
- db: mysql
# You typically want _self_ somewhere after the schema (base_config)
- _self_

debug: true
38 changes: 36 additions & 2 deletions hydra/_internal/defaults_list.py
@@ -1,6 +1,8 @@
# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved

import copy
import os
import warnings
from dataclasses import dataclass, field
from textwrap import dedent
from typing import Callable, Dict, List, Optional, Set, Tuple, Union
Expand Down Expand Up @@ -218,7 +220,11 @@ class DefaultsList:
overrides: Overrides


def _validate_self(containing_node: InputDefault, defaults: List[InputDefault]) -> bool:
def _validate_self(
containing_node: InputDefault,
defaults: List[InputDefault],
has_config_content: bool,
) -> bool:
# check that self is present only once
has_self = False
has_non_override = False
Expand All @@ -233,6 +239,16 @@ def _validate_self(containing_node: InputDefault, defaults: List[InputDefault])
has_self = True

if not has_self and has_non_override or len(defaults) == 0:
# This check is here to make the migration from Hydra 1.0 to Hydra 1.1 smoother and should be removed in 1.2
# The warning should be removed in 1.2
if containing_node.primary and has_config_content and has_non_override:
msg = (
f"In '{containing_node.get_config_path()}': Defaults list is missing `_self_`. "
f"See https://hydra.cc/docs/upgrades/1.0_to_1.1/default_composition_order for more information"
)
if os.environ.get("SELF_WARNING_AS_ERROR") == "1":
raise ConfigCompositionException(msg)
warnings.warn(msg, UserWarning)
defaults.append(ConfigDefault(path="_self_"))

return not has_self
Expand Down Expand Up @@ -413,6 +429,16 @@ def _update_overrides(
overrides.add_override(parent.get_config_path(), d)


def _has_config_content(cfg: DictConfig) -> bool:
if cfg._is_none() or cfg._is_missing():
return False

for key in cfg.keys():
if not OmegaConf.is_missing(cfg, key):
return True
return False


def _create_defaults_tree_impl(
repo: IConfigRepository,
root: DefaultsTreeNode,
Expand Down Expand Up @@ -469,7 +495,15 @@ def _create_defaults_tree_impl(
or is_root_config
and len(overrides.append_group_defaults) > 0
):
self_added = _validate_self(containing_node=parent, defaults=defaults_list)
has_config_content = isinstance(
loaded.config, DictConfig
) and _has_config_content(loaded.config)

self_added = _validate_self(
containing_node=parent,
defaults=defaults_list,
has_config_content=has_config_content,
)

if is_root_config:
defaults_list.extend(overrides.append_group_defaults)
Expand Down
1 change: 1 addition & 0 deletions hydra/test_utils/configs/completion_test/config.yaml
@@ -1,4 +1,5 @@
defaults:
- _self_
- group: null

# a mapping item
Expand Down
1 change: 1 addition & 0 deletions news/1755.feature
@@ -0,0 +1 @@
To make migration from Hydra 1.0 to 1.1 easier, Hydra will now issue a warning if the primary config defines config values and Defaults List if the Defaults List does not specify `_self_`
1 change: 1 addition & 0 deletions plugins/hydra_ax_sweeper/tests/config/config.yaml
@@ -1,4 +1,5 @@
defaults:
- _self_
- quadratic: null
- params: null
- override hydra/sweeper: ax
Expand Down
1 change: 1 addition & 0 deletions tests/test_apps/schema-overrides-hydra/config.yaml
Expand Up @@ -7,4 +7,5 @@ hydra:

defaults:
- config_schema
- _self_
- group: a
1 change: 1 addition & 0 deletions tools/configen/README.md
Expand Up @@ -27,6 +27,7 @@ This will generate a basic config similar to:
$ cat conf/configen.yaml
defaults:
- configen_schema
- _self_

configen:
# output directory
Expand Down
1 change: 1 addition & 0 deletions tools/configen/conf/configen.yaml
@@ -1,5 +1,6 @@
defaults:
- configen_schema
- _self_

configen:
# output directory
Expand Down
2 changes: 1 addition & 1 deletion tools/configen/configen/configen.py
Expand Up @@ -237,7 +237,7 @@ def generate_module(cfg: ConfigenConf, module: ModuleConf) -> str:
)


@hydra.main(config_name="configen_schema")
@hydra.main(config_path=None, config_name="configen_schema")
def main(cfg: Config):
if cfg.init_config_dir is not None:
init_config(cfg.init_config_dir)
Expand Down
1 change: 1 addition & 0 deletions tools/configen/configen/templates/sample_config.yaml
@@ -1,5 +1,6 @@
defaults:
- configen_schema
- _self_

configen:
# output directory
Expand Down
1 change: 1 addition & 0 deletions tools/configen/example/config.yaml
@@ -1,5 +1,6 @@
defaults:
- config_schema
- _self_

user:
age: 7
Expand Down
5 changes: 4 additions & 1 deletion tools/configen/setup.py
Expand Up @@ -12,5 +12,8 @@
author_email="omry@fb.com, rosario@cs.uw.edu",
url="https://github.com/facebookresearch/hydra/tree/master/tools/configen",
include_package_data=True,
install_requires=["hydra-core>=1.1.0dev1", "jinja2"],
install_requires=[
"hydra-core>=1.1.0",
"jinja2",
],
)
67 changes: 67 additions & 0 deletions website/docs/tutorials/basic/your_first_app/5_defaults.md
Expand Up @@ -68,6 +68,73 @@ $ python my_app.py ~db
{}
```

### Composition order of primary config
Your primary config can contain both config values and a Defaults List.
In such cases, you should add the `_self_` keyword to your defaults list to specify the composition order of the config file relative to the items in the defaults list.

* If you want your primary config to override the values of configs from the Defaults List, append `_self_` to the end of the Defaults List.
* If you want the configs from the Defaults List to override the values in your primary config, insert `_self_` as the first item in your Defaults List.


<div className="row">

<div className="col col--6">

```yaml title="config.yaml" {3}
defaults:
- db: mysql
- _self_

db:
user: root
```
</div>
<div className="col col--6">

```yaml title="Result config: db.user from config.yalm" {4}
db:
driver: mysql # db/mysql.yaml
pass: secret # db/mysql.yaml
user: root # config.yaml


```
</div>
<div className="col col--6">

```yaml title="config.yaml" {2}
defaults:
- _self_
- db: mysql

db:
user: root
```
</div>
<div className="col col--6">

```yaml title="Result config: All values from db/mysql" {4}
db:
driver: mysql # db/mysql.yaml
pass: secret # db/mysql.yaml
user: omry # db/mysql.yaml


```
</div>
</div>

See [Compositon Order](advanced/defaults_list.md#composition-order) for more information.

:::info
The default composition order changed between Hydra 1.0 and Hydra 1.1.
- **Hydra 1.0**: Configs from the defaults list are overriding the primary config
- **Hydra 1.1**: A config is overriding the configs from the defaults list.

To mitigate confusion, Hydra 1.1 issue a warning if the primary config contains both Default List and Config values, and `_self_` is not specified in the Defaults List.
The warning will disappear if you add `_self_` to the Defaults List based on the desired behavior.
:::

### Non-config group defaults
Sometimes a config file does not belong in any config group.
You can still load it by default. Here is an example for `some_file.yaml`.
Expand Down
34 changes: 32 additions & 2 deletions website/docs/tutorials/structured_config/4_defaults.md
Expand Up @@ -68,7 +68,37 @@ db:
...
```

#### Requiring users to specify a default list value
### A Note about composition order
The default composition order in Hydra is that values defined in a config are merged into values introduced from configs in the Defaults List - or in other words - overriding them.
This behavior can be unintuitive when your primary config is a Structured Config, like in the example above.
For example, if the primary config is:
```python {6}
@dataclass
class Config:
defaults: List[Any] = field(default_factory=lambda: [
"debug/activate",
# If you do not specify _self_, it will be appended to the end of the defaults list by default.
"_self_"
])

debug: bool = False
```
And `debug/activate.yaml` is overriding the `debug` flag to `True`, the composition order would be such that debug ends up being `False`.
To get `debug/activate.yaml` to override this config, explicitly specify `_self_` before `debug/activate.yaml`:
```python {4}
@dataclass
class Config:
defaults: List[Any] = field(default_factory=lambda: [
"_self_",
"debug/activate",
])

debug: bool = False
```

See [Compositon Order](advanced/defaults_list.md#composition-order) for more information.

### Requiring users to specify a default list value

Set `db` as `MISSING` to require the user to specify a value on the command line.

Expand Down Expand Up @@ -97,4 +127,4 @@ Available options:


</div>
</div>
</div>
16 changes: 16 additions & 0 deletions website/docs/tutorials/structured_config/5_schema.md
Expand Up @@ -37,6 +37,8 @@ Then, we will use the Defaults List in each config to specify its base config as
defaults:
- base_config
- db: mysql
# See composition order note
- _self_

debug: true
```
Expand All @@ -50,6 +52,8 @@ defaults:

user: omry
password: secret


```
</div>
<div className="col col--4">
Expand All @@ -60,6 +64,8 @@ defaults:

user: postgres_user
password: drowssap


```
</div>
</div>
Expand Down Expand Up @@ -168,6 +174,7 @@ Defaults List

</details>


### Validating against a schema from a different config group

<ExampleGithubLink to="examples/tutorials/structured_configs/5.2_structured_config_schema_different_config_group"/>
Expand Down Expand Up @@ -261,6 +268,8 @@ password: secret
```yaml title="db/postgresql.yaml" {2}
defaults:
- /database_lib/db/postgresql@_here_
# See composition order note
- _self_

user: postgres_user
password: drowssap
Expand All @@ -272,3 +281,10 @@ password: drowssap
- we override the package to `_here_` to ensure that the package of the schema is the same as the package
of the config it's validating.

### A Note about composition order
By default, Hydra 1.1 appends `_self_` to the end of the Defaults List.
This behavior is new in Hydra 1.1 and different from previous Hydra versions. As such Hydra 1.1 issues a warning if `_self_` is not specified **in the primary config**, asking you to add `_self_` and thus indicate the desired composition order.
To address the warning while maintaining the new behavior, append `_self_` to the end of the Defaults List. Note that in some cases it may instead be desirable to add `_self_` directly after the schema and before other Defaults List elements.


See [Compositon Order](advanced/defaults_list.md#composition-order) for more information.

0 comments on commit b4a5bc2

Please sign in to comment.