Multipass support#350
Conversation
|
I will split up the tex -> html/pdf into another PR. This PR is ready for review now |
gkreitz
left a comment
gkreitz
left a comment
There was a problem hiding this comment.
Looks good. Added two nit-picky comments, but they are so small I'll just merge this. We can fix those in a future PR if we want/care.
| if interaction.startswith('---'): | ||
| passes.append(curr_pass) | ||
| curr_pass = [] | ||
| continue |
There was a problem hiding this comment.
This continue could probably be an else (and I think that'd be preferable)
| return SubmissionResult('JE', reason=f'failed to parse validator score: {e}') | ||
| else: | ||
| return SubmissionResult('JE', reason='problem has custom scoring but validator did not produce "score.txt"') | ||
| # If we're running multipass, we do not need to output a score after every pass |
There was a problem hiding this comment.
Do you actually need this check? Multi-pass can only happen in the new format, where custom score is never mandatory. So I think you could have left this else-clause as-is?
(If we do in fact need it, can't we just check both that the problem is multipass and that nextpass.in exists, making the check correct and avoiding a scary comment about accepting a small risk? :)
There was a problem hiding this comment.
Yes, that's true. I somehow missed that we actually have a reference to the metadata via self.problem in this class. Will add both of these in the multipass sample PR.
2 participants
I dislike the inline functions on line 415 and 421, but ruff demands it...
I do not try to prevent the submission from passing information out of bound. In my opinion, doing so would just be cosmetic, as it's impossible to do it fully without building a sandbox.
In a follow-up PR, I will fix multipass sample rendering when the source is tex. I have fixed it for md -> html in this PR.