You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
At a high level, this is obviously more a design question than about omegaconf specifically, i.e. I don't see what omegaconf could do here to help, but I'd love to discuss it and ask for ideas about better ways to do what I want to do. I think I have a neat solution, with the problem that it's possibly too neat and magic, and perhaps I should do something different (I'll give you the code below). After all, I doubt my problem is uncommon. Even if there's no better approach, perhaps someone can get some good ideas from my current code.
So, I'd like to solicit your ideas or insight on what is the best way to solve it. I'm open to radically different solutions. Are there (arguably) better ways to solve this?
Problem
I want to use values loaded from config as default values of function arguments:
deffoo(arg: int=config.default_arg) ->None: ...
Now this is obviously possible with omegaconf if we load the config at module import time, but for many reasons (e.g. testability) I'd actually very much like to avoid this.
I find it gets very unwieldy to everywhere do things like
Avoid loading config at import time, so that for example my tests do not depend on a config
Use a value from the config for a (module level) function default parameter.
In Python, default values are evaluated at import time, so this obviously is not quite possible exactly as stated.
My current solution
This feels too magic, but kind of works. What I like is that it avoids the repetitive arg = ... if arg is None else ... mess. It allows me to achieve what I want using syntax like this:
The wrapper uses_config replaces FromConfig default values with values from the config at first call time (not wrapper creation time), at a point where the configuration should already be loaded.
See Appendix for the code implementing this.
Discussion
There are certainly things that I don't like here, starting from how magic this is; the fact that I lose some type safety (though I think the wrapper could be modified to compare the type annotations); and that I need to remember to use @uses_config.
I think one alternative approach, with its own downsides, would be to get rid of the global config object entirely (globals kind of suck, not least for testability), make load return a _Config and then pass that to everything. I don't immediately see how this would help me get rid of the magic, but it would allow us to get rid of the delicate "do not call this at import time" pitfall.
I also considered using typing.Annotations to mark the arguments that should have default values taken from the config, but it feels like that doesn't really make it easier in any way.
My initial approach actually involved creating a _Config object named fromConfig with the same hierarchy as a real config, but with all the leaf values replaced by objects that told the path to the config. Then you could say
and even have type safety. Unfortunately, creating such a class to mirror an attrs class (with some validation) turned out to be a bit tricky, and of course it still won't work in expressions like fromConfig.backend.port + 1. Lists and dictionaries of items in the config seem to be tricky, too.
Appendix: Implementing code
importcontextlibimportfunctoolsimportinspectfrompathlibimportPathfromtypingimportIO, Any, Callable, TypeVar, castimportattrsfromloguruimportloggerfromomegaconfimportOmegaConfCONFIG_PATH=Path("config.yaml")
@attrs.defineclassDocumentSync:
"""An example config class."""data_gen_path: Pathsource_docs_path: Pathparsed_docs_path: Path@attrs.defineclassBackend:
"""An example config class."""listen_port: int@attrs.defineclass_Config:
"""An example top level config class."""backend: Backenddocument_sync: DocumentSyncclassConfigurationNotLoadedError(Exception):
"""Raised when the configuration is not loaded yet when trying to access it."""class_ConfigProxy:
""" A proxy for the config object before the config is loaded. At attribute access time, checks if the config has since been loaded (can happen if this is "imported from" before the config is loaded) and forwards the attribute access to the config object. """def__getattribute__(self, name: str) ->Any:
globalconfigifname.startswith("__"):
returnsuper().__getattribute__(name)
# This can happen if this has been "imported from" before the config was loadedifisinstance(config, _Config):
returngetattr(config, name)
logger.error("Configuration not loaded yet")
raiseConfigurationNotLoadedError("Configuration not loaded yet")
def_is_import_in_progress() ->bool:
returnany("importlib._bootstrap"inframe_info.filenameforframe_infoininspect.stack())
# The config object to be used to access the config.config: _Config=cast(_Config, _ConfigProxy())
defload(
*,
reload: bool=False,
config_file: Path|IO[Any] =CONFIG_PATH,
_allow_module_level: bool=False, # for testing only
) ->None:
""" (Re)load the configuration. Do not call from module level (at import time). """globalconfigifisinstance(config, _Config) andnotreload:
returnifnot_allow_module_leveland_is_import_in_progress():
raiseConfigurationNotLoadedError("Configuration loaded during import")
config=cast(
_Config, OmegaConf.to_object(OmegaConf.merge(OmegaConf.structured(_Config), OmegaConf.load(config_file)))
)
@attrs.frozenclass_FromConfig:
"""A marker for a function argument that should be replaced by a value from the config."""key: strFromConfig: Callable[[str], Any] =_FromConfig@functools.cachedef_get_config_item(path: str) ->Any:
cfg=configforpartinpath.split("."):
cfg=getattr(cfg, part)
returncfg_T=TypeVar("_T")
def_check_config_key(path: str) ->None:
cls: type[Any] =_Configforpartinpath.split("."):
withcontextlib.suppress(attrs.exceptions.NotAnAttrsClassError):
cls=getattr(attrs.fields(cls), part).typedefuses_config(func: Callable[..., _T]) ->Callable[..., _T]:
"""Replace uses of FromConfig by values from config."""config_args: dict[str, Any] |None=None# Validate the config keys at wrapper creation timeforarg_valueininspect.signature(func).parameters.values():
ifisinstance(arg_value.default, _FromConfig):
_check_config_key(arg_value.default.key)
definit() ->None:
# Initialize the config args at first call timenonlocalconfig_argsif_is_import_in_progress():
raiseConfigurationNotLoadedError("Function with @uses_config decorator called during import")
config_args= {}
forarg_name, arg_valueininspect.signature(func).parameters.items():
ifisinstance(arg_value.default, _FromConfig):
config_args[arg_name] =_get_config_item(arg_value.default.key)
@functools.wraps(func)defwrapper(*args: tuple[Any, ...], **kwargs: dict[str, Any]) ->_T:
ifconfig_argsisNone:
init()
assertconfig_argsisnotNonekwargs= {**config_args, **kwargs}
returnfunc(*args, **kwargs)
returnwrapper
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
At a high level, this is obviously more a design question than about omegaconf specifically, i.e. I don't see what omegaconf could do here to help, but I'd love to discuss it and ask for ideas about better ways to do what I want to do. I think I have a neat solution, with the problem that it's possibly too neat and magic, and perhaps I should do something different (I'll give you the code below). After all, I doubt my problem is uncommon. Even if there's no better approach, perhaps someone can get some good ideas from my current code.
So, I'd like to solicit your ideas or insight on what is the best way to solve it. I'm open to radically different solutions. Are there (arguably) better ways to solve this?
Problem
I want to use values loaded from config as default values of function arguments:
Now this is obviously possible with omegaconf if we load the config at module import time, but for many reasons (e.g. testability) I'd actually very much like to avoid this.
I find it gets very unwieldy to everywhere do things like
So, to recap, I would like to have both:
In Python, default values are evaluated at import time, so this obviously is not quite possible exactly as stated.
My current solution
This feels too magic, but kind of works. What I like is that it avoids the repetitive
arg = ... if arg is None else ...
mess. It allows me to achieve what I want using syntax like this:The wrapper
uses_config
replacesFromConfig
default values with values from the config at first call time (not wrapper creation time), at a point where the configuration should already be loaded.See Appendix for the code implementing this.
Discussion
There are certainly things that I don't like here, starting from how magic this is; the fact that I lose some type safety (though I think the wrapper could be modified to compare the type annotations); and that I need to remember to use
@uses_config
.I think one alternative approach, with its own downsides, would be to get rid of the global
config
object entirely (globals kind of suck, not least for testability), makeload
return a_Config
and then pass that to everything. I don't immediately see how this would help me get rid of the magic, but it would allow us to get rid of the delicate "do not call this at import time" pitfall.I also considered using
typing.Annotation
s to mark the arguments that should have default values taken from the config, but it feels like that doesn't really make it easier in any way.My initial approach actually involved creating a
_Config
object namedfromConfig
with the same hierarchy as a real config, but with all the leaf values replaced by objects that told the path to the config. Then you could sayand even have type safety. Unfortunately, creating such a class to mirror an attrs class (with some validation) turned out to be a bit tricky, and of course it still won't work in expressions like
fromConfig.backend.port + 1
. Lists and dictionaries of items in the config seem to be tricky, too.Appendix: Implementing code
Beta Was this translation helpful? Give feedback.
All reactions