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

assign_based_on assignments to recipe inputs not reflected in summary #264

Open
o-smirnov opened this issue Mar 19, 2024 · 4 comments
Open
Assignees
Labels
bug Something isn't working
Milestone

Comments

@o-smirnov
Copy link
Member

The assignments work correctly, but the summary of recipe inputs that is being printed up front prints the default values without the assignments being applied. This is a cosmetic issue, but can be confusing.

@o-smirnov o-smirnov self-assigned this Mar 19, 2024
@o-smirnov
Copy link
Member Author

Spotted by @JSKenyon.

Not so quick to fix. It's an unholy interaction between defaults and assign_based_on. The latter changes the default value of a recipe input (as opposed to the input itself), reason being that it still gives the user the opportunity to override assign_based_on with an explicit command-line setting. But assign_based_on is not finally applied until the run() stage. At the validation stage (where the summary is printed), it hasn't quite taken hold yet.

@o-smirnov
Copy link
Member Author

Problem arises here, non-assigned defaults sneak in:

for name, value in self.defaults.items():

But commenting it out breaks some substitutions. Need to rethink this a little -- not going to worry for R2.0.

@o-smirnov
Copy link
Member Author

Yep, the deeper issue is that parameters are prevalidated, then revalidated again here, which causes {}-substitution to happen twice. The breakage case is with publish_plots_title parameter of the PARROT recipe, which uses "{{" escapes. The code referenced in the previous comment inadvertently fixes this bug, but only in the case where the "{{" containing strings come from a default. I can probably devise another way to break it.

The validation logic needs to be cleaned up a little to absolutely avoid double substitution. Deferring this until post-2.0.

@o-smirnov
Copy link
Member Author

Note also related to this -- stimela run image-parrot.yml obs=L4 -s flag-reset initial-flag-version='xxx'

2024-03-27 12:25:28 STIMELA.ratt-parrot.flag-reset ERROR: step 'flag-reset': invalid inputs: 'versionname'    

@o-smirnov o-smirnov added the bug Something isn't working label Apr 29, 2024
@o-smirnov o-smirnov added this to the R2.0.1 milestone Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant