-
Notifications
You must be signed in to change notification settings - Fork 27
#487 Implement new sample override #488
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
base: main
Are you sure you want to change the base?
Conversation
|
@thorehusfeldt can you check the schema again? ^^' |
mpsijm
left a comment
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.
Quick code review, didn't test it yet. The CUE schema looks good on first glance (maybe I'd put "ans" before "ans.statement" for consistency, but that's a nit). Note that the JSON schema should also be updated.
| in_file, ans_file = sample.download | ||
| base_name = outputdir / str(i + 1) | ||
| contents[base_name.with_suffix(".in")] = in_file | ||
| if in_file.stat().st_size > 0: |
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.
Does it ever happen that we can only download a .ans file? I'd say the .in file is required, but maybe I missed something in the discussion.
Same on line 357.
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.
a .ans file without a .in file does not really make sense... but having neither might be useful?
|
@mpsijm @thorehusfeldt can you check the schemas? |
Closes #487.
Try to implement new sample data behaviour