Force fuzzyspec expansion to always resolve the ns #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a namespace depends on a production rule (e.g. one that returns a dictionary or list of dictionaries that are then used in collect) the namespace would be resolved once but then be oblivious to the changes in the input of the production rule, that would make it required to reevaluate it. Rather than adding more special cases, just force all keys to be revaluated by the config object so as to keep them up to date within the current namespace. I cannot think of many downsides to this change, other than having to needlessly reevaluate a few inputs (which are assumed to be cheap in the first place). The test in test_complexinput.py represents a minimal example of input that would fail in master and work correctly with this change.
Replace old tests that rely on the behaviour of expand_partial.
|
Unfortunately this doesn't help with #38, which I am yet to understand. |
This fails because the collect node is resolved at compile time which works differently.
This avoids and edge case where collect is resolved at compile time (because we are collecting over a production rule) and the dependencies have changed inadvertently. To remedy that we simply force the node to always be reevaluated.
|
Will review with high priority because this breaks a lot of stuff in vp |
|
This may take me a while to understand. I always accepted |
wilsonmr
approved these changes
May 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a namespace depends on a production rule (e.g. one that returns
a dictionary or list of dictionaries that are then used in collect) the
namespace would be resolved once but then be oblivious to the changes in
the input of the production rule, that would make it required to
reevaluate it.
Rather than adding more special cases, just force all keys to be
revaluated by the config object so as to keep them up to date within the
current namespace. I cannot think of many downsides to this change,
other than having to needlessly reevaluate a few inputs (which are
assumed to be cheap in the first place).
The test in test_complexinput.py represents a minimal example of input
that would fail in master and work correctly with this change.