Skip to content

Commit

Permalink
Fix a heisenbug in configs when unpacking a dictionary config (#2185)
Browse files Browse the repository at this point in the history
* Fix a heisenbug in configs when unpacking a dictionary config

* Cleaner fix

* Hopefully fix issues -- this is *grrr*
  • Loading branch information
romain-intel authored Jan 7, 2025
1 parent a7042c7 commit 0dfef56
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
2 changes: 1 addition & 1 deletion metaflow/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ def init(self, ignore_errors=False):
# Resolve any value from configurations
self.kwargs = unpack_delayed_evaluator(self.kwargs, ignore_errors=ignore_errors)
# Do it one item at a time so errors are ignored at that level (as opposed to
# at the entire kwargs leve)
# at the entire kwargs level)
self.kwargs = {
k: resolve_delayed_evaluator(v, ignore_errors=ignore_errors)
for k, v in self.kwargs.items()
Expand Down
29 changes: 23 additions & 6 deletions metaflow/user_configs/config_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def __len__(self):

def __getattr__(self, name):
if self._access is None:
raise AttributeError()
raise AttributeError(name)
self._access.append(name)
return self

Expand Down Expand Up @@ -336,6 +336,8 @@ def __init__(
self.parser = parser
self._computed_value = None

self._delayed_evaluator = None

def load_parameter(self, v):
if v is None:
return None
Expand All @@ -344,22 +346,37 @@ def load_parameter(self, v):
def _store_value(self, v: Any) -> None:
self._computed_value = v

def _init_delayed_evaluator(self) -> None:
if self._delayed_evaluator is None:
self._delayed_evaluator = DelayEvaluator(self.name.lower())

# Support <config>.<var> syntax
def __getattr__(self, name):
return DelayEvaluator(self.name.lower()).__getattr__(name)
# Need to return a new DelayEvaluator everytime because the evaluator will
# contain the "path" (ie: .name) and can be further accessed.
return getattr(DelayEvaluator(self.name.lower()), name)

# Next three methods are to implement mapping to support **<config> syntax
# Next three methods are to implement mapping to support **<config> syntax. We
# need to be careful, however, to also support a regular `config["key"]` syntax
# which calls into `__getitem__` and therefore behaves like __getattr__ above.
def __iter__(self):
return iter(DelayEvaluator(self.name.lower()))
self._init_delayed_evaluator()
yield from self._delayed_evaluator

def __len__(self):
return len(DelayEvaluator(self.name.lower()))
self._init_delayed_evaluator()
return len(self._delayed_evaluator)

def __getitem__(self, key):
self._init_delayed_evaluator()
if key.startswith(UNPACK_KEY):
return self._delayed_evaluator[key]
return DelayEvaluator(self.name.lower())[key]


def resolve_delayed_evaluator(v: Any, ignore_errors: bool = False) -> Any:
# NOTE: We don't ignore errors in downstream calls because we want to have either
# all or nothing for the top-level call by the user.
try:
if isinstance(v, DelayEvaluator):
return v()
Expand Down Expand Up @@ -397,7 +414,7 @@ def unpack_delayed_evaluator(
else:
# k.startswith(UNPACK_KEY)
try:
result.update(resolve_delayed_evaluator(v[k]))
result.update(resolve_delayed_evaluator(v))
except Exception as e:
if ignore_errors:
continue
Expand Down

0 comments on commit 0dfef56

Please sign in to comment.