Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make select and oc.select more robust #1127

Open
SZiesche opened this issue Sep 26, 2023 · 2 comments
Open

Make select and oc.select more robust #1127

SZiesche opened this issue Sep 26, 2023 · 2 comments

Comments

@SZiesche
Copy link

SZiesche commented Sep 26, 2023

Is your feature request related to a problem? Please describe.
By using oc.select we can define default values if the interpolation target is not found. However, the "not found" part is not very robust. For example, if we refer to a non-existing node, we still raise an error.

Consider for example the case, where we have a config, that is usually nested in some other config and has a member like my_member: int = ${oc.select:..member, 5} that is by default inherited from the parent. Now we want to test this config standalone or we want to use it in a different context were the parent doesn't exist, then the code just crashes.

Describe the solution you'd like
It would be cool to turn this off or to give the user an option to turn this off. For example, we could change the select implementation to

def select_value(
    cfg: Container,
    key: str,
    *,
    default: Any = _DEFAULT_MARKER_,
    throw_on_resolution_failure: bool = True,
    throw_on_missing: bool = False,
    absolute_key: bool = False,
) -> Any:
    try:  # this try/except block is new, the rest stays the same.
        node = select_node(
            cfg=cfg,
            key=key,
            throw_on_resolution_failure=throw_on_resolution_failure,
            throw_on_missing=throw_on_missing,
            absolute_key=absolute_key,
        )
    except ConfigKeyError as e:
        node = None

    node_not_found = node is None
    if node_not_found or node._is_missing():
        if default is not _DEFAULT_MARKER_:
            return default
        else:
            return None

    return _get_value(node)

Describe alternatives you've considered
Alternatively, we could add a flag like throw_on_node_not_existing to the function and raise the error depending on that value. Then the users could relatively easy write their own robust select method.

@odelalleau
Copy link
Collaborator

Indeed, it shouldn't crash if it can't find the node when a default value is provided.

@SZiesche
Copy link
Author

SZiesche commented Sep 27, 2023

to make this more precise, I would like to have the following script pass.

from dataclasses import dataclass

from omegaconf import OmegaConf, II


cfg1 = OmegaConf.create({"a": "${oc.select:..member, 5}"})
print(cfg1.a)  # this raises an Error because of the missing node


@dataclass
class Config:
    a: int = II("${oc.select:..member, 5}")


cfg2 = OmegaConf.structured(Config)
print(cfg2.a)  # this raises an Error too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants