-
Notifications
You must be signed in to change notification settings - Fork 4
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
Attempt at making substitutions work inside dicts and lists. #269
Conversation
scabha/evaluator.py
Outdated
recursive, | ||
verbose | ||
) | ||
elif isinstance(value, list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JSKenyon clever, clever. Maybe use (list, ListConfig)
just in case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better:
params_out[name] = type(value)([x for x in
self.evaluate_dict({f"[{i}]": v for i, v in enumerate(value)}, ...).values()
])
...because then any nested error messages will report the location of the error sensibly as "[n]".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, awesome. Definitely better than a profusion of dummies
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no need for the extra list comprehension, right? This should work too:
type(value)(self.evaluate_dict({f"[{i}]": v for i, v in enumerate(value)}, ...).values())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have made a final change. Let me know if it appeals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, also the evaluate_dict()
call needs sublocation=sublocation + [name]
, so that errors are reported from the correct nested namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making those changes @o-smirnov - was only going to get to it today.
This PR aims to improve substitutions by supporting them inside compound types. This probably needs extensive testing, but the following should now work:
or equivalently:
This should also support more complicated cases e.g.: