Enhance instructions (Elena suggestions)#32
Conversation
| score_log = "score.py log" | ||
| setup = "human_setup.py" | ||
| submit = "submit.py" | ||
| submit = "submit.py ''" |
There was a problem hiding this comment.
My intuition is to add a default value for submit.py, such as maybe:
def main(submission: str = ""):
or
@click.argument("SUBMISSION", type=str, default="")
rather than sending the default value here
Also I suspect this change you made wouldn't allow the user to actually submit something
I didn't test either theory
I am not actually pushing back, feel free to ignore
There was a problem hiding this comment.
did you test if this works, still trying to submit something?
my intuition is "I wish we had tests", but also "if/when we work together I expect we'll write tests as we go"
There was a problem hiding this comment.
Not tested, agree I really should!
Re: design of this, I'd be happy with anything that allowed user to simply enter submit in terminal.
There was a problem hiding this comment.
- This is not the right place to make this change
- We should not let the user "simply enter
submit" without confirming that they want to submit the empty string (e.g. usingclick.prompt). Some tasks require a non-empty submission (though there have been talks about changing that).
| async def introduction(run_info: dict): | ||
| commands = { | ||
| HelperCommand.clock.name: "Start and pause the timer, or see elapsed time.", | ||
| HelperCommand.clock.name: "Start and pause the timer, or see elapsed time. If you reload or otherwise close your terminal, you will need to run this command again.", |
There was a problem hiding this comment.
My intuition is to print exactly how to run the command, so the user can later copy it
There was a problem hiding this comment.
This would end up being very repetitive, since it should literally be just the command name in each case.
There was a problem hiding this comment.
I'm not sure this new sentence is correct. The clock isn't automatically stopped or anything if you "reload or otherwise close your terminal", so you don't need to run clock again. I don't understand what this sentence is referring to.
| score_log = "score.py log" | ||
| setup = "human_setup.py" | ||
| submit = "submit.py" | ||
| submit = "submit.py ''" |
There was a problem hiding this comment.
- This is not the right place to make this change
- We should not let the user "simply enter
submit" without confirming that they want to submit the empty string (e.g. usingclick.prompt). Some tasks require a non-empty submission (though there have been talks about changing that).
| async def introduction(run_info: dict): | ||
| commands = { | ||
| HelperCommand.clock.name: "Start and pause the timer, or see elapsed time.", | ||
| HelperCommand.clock.name: "Start and pause the timer, or see elapsed time. If you reload or otherwise close your terminal, you will need to run this command again.", |
There was a problem hiding this comment.
I'm not sure this new sentence is correct. The clock isn't automatically stopped or anything if you "reload or otherwise close your terminal", so you don't need to run clock again. I don't understand what this sentence is referring to.
|
|
||
| click.confirm( | ||
| f"Do you definitely want to end the task and submit '{submission}'?", | ||
| f"Do you definitely want to end the task and submit '{submission}'? (You will lose access to the task environment.)", |
There was a problem hiding this comment.
I think this can be formatted better:
click.echo("You are about to end the task and submit '{submission}'")
click.echo("Your work will be scored, and then you will be disconnected from the task environment")
click.confirm("Are you sure you want to proceed?", abort=True)
|
I went and implemented these comments |
|
Tests are failing. Also, please don't ping, instead request re-review. |
| async def introduction(run_info: dict) -> tuple[str, str, str]: | ||
| commands = { | ||
| HelperCommand.clock.name: "Start and pause the timer, or see elapsed time.", | ||
| HelperCommand.clock.name: "Start and pause the timer, or see elapsed time. This must be run explicitly - reloading or otherwise closing your terminal will not change the clock status.", |
There was a problem hiding this comment.
I don't understand what the second sentence means.
4 participants
No description provided.