-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add 'submission_id' and 'Kill Sub' columns in New submissions leaderboard #589
Conversation
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.
Thanks @frcaud! That's the idea, though we need to figure out why CI fails and fix it.
My first guess (looking at the last history commit in |
I see that The Line 11 in 9b1d738
|
It might be due to pytest-dev/pytest#11261 |
You need to add an empty |
Thanks @glemaitre ! |
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.
Thanks @frcaud ! For the failing test we need to edit test_get_leaderboard
to add the extra column we added in,
> assert (
"""<th>team</th>
<th>submission</th>
<th>submitted at (UTC)</th>
<th>error</th>"""
in leaderboard_failed
)
assertion. The pytest log is a bit confusing, but I think that's actually the only failing test. The other exceptions you see in the logs, are the expected exceptions that are obtained when running a bad submission and can be ignored.
A few more comments below, otherwise LGTM.
@@ -427,12 +439,19 @@ def get_leaderboard( | |||
columns = [ | |||
"team", | |||
"submission", | |||
"submission_id", |
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.
Maybe let's just call it id
so it would be a less wide column in the resulting html table? It's shown under a section of New submission, so there is no need to repeat "submission" in column names I think.
return f"<button onclick=\"alert(('You killed ' \ | ||
+ 'submission {cell_value}!'))\">{cell_value}</button>" | ||
|
||
df["Kill Sub"] = df["submission_id"].apply( |
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.
Again just "Kill" or maybe better "Stop" would be enough. It's in a table of submissions already.
competition_type = "public" if "public" in leaderboard_type else "private" | ||
competition_type = ( | ||
"public" if "public" in leaderboard_type else "private" | ||
) |
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.
I think if leaderboard_type not in ['new', 'failed']
we need to delete the extra column "id" we created before _compute_competition_leaderboard
is run otherwise it will show in all other leaderboards
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.
Since we create this extra column in elif leaderboard_type in ["new", "failed"]:
it seems that we don't need to delete it in other leaderboard types, or am I mistaken ?
Thanks @rth ! Tests OK. Seems there is one more error with codecov. |
CIs are green on rerun. Thanks @glemaitre and @rth ! |
Thanks! Linting CI is still failing though. Can you run black on it? |
Adding a Kill Submission button in New submissions leaderboard to be able to terminate a submission.
First PR is adding a 'submission_id' column and adding a 'Kill Sub' column with clickable buttons on the HTML rendering of the New submissions leaderboard (leaderboard and My submissions pages on frontend)