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

Codechef atcoder support #470

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

iampjeetsingh
Copy link

Implemented the following ;handle identify, ;handle set, ;handle list, ;ranklist commands for CodeChef / AtCoder / Google (Kickstart)

@cheran-senthil
Copy link
Owner

cheran-senthil commented Jul 22, 2021

Don't think we want these for other sites, general consensus is against this.

@algmyr algmyr reopened this Jul 22, 2021
@cheran-senthil cheran-senthil requested a review from algmyr July 22, 2021 14:24
@krofna
Copy link
Collaborator

krofna commented Jul 22, 2021

#94 for reference

@iampjeetsingh
Copy link
Author

Which features were you talking in #468 then?

@krofna
Copy link
Collaborator

krofna commented Jul 22, 2021

@iampjeetsingh just ignore what cheran said. this requires detailed discussion. please provide your discord username so that we can invite you to the development server

@iampjeetsingh
Copy link
Author

My discord username is thesupremeone#1162

@krofna krofna requested review from meooow25 and krofna August 8, 2021 21:06
@@ -84,6 +85,7 @@ Fill in appropriate variables in new "environment" file.
- **ALLOW_DUEL_SELF_REGISTER**: boolean value indicating if self registration for duels is enabled.
- **TLE_ADMIN**: the name of the role that can run admin commands of the bot. If this is not set, the role name will default to "Admin".
- **TLE_MODERATOR**: the name of the role that can run moderator commands of the bot. If this is not set, the role name will default to "Moderator".
- **CLIST_API_TOKEN**: Credential for accessing clist api, You can find your api key [here][https://clist.by/api/v2/doc/] after creating an account on clist.by. If this is not set, codechef/atcoder/google(kickstart) related commands won't work.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Markdown links are [label](url), with parentheses.

"""Shows ranklist for the contest with given contest id. If handles contains
'+server', all server members are included. No handles defaults to '+server'.

You can frame contest_id as follow
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"You can specify contest_id as follows"?

Use QR for Qualification Round and WF for World Finals.

# If nothing works
Use clist contest_id. You have to prefix - sign to clist contest-id otherwise it will be considered a codeforces contest id.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we consider doing this with a clist prefix rather than a minus sign? The minus way seems like a hack to me.

async def _show_ranklist(self, channel, contest_id: int, handles: [str], ranklist, vc: bool = False, delete_after: float = None):
contest = cf_common.cache2.contest_cache.get_contest(contest_id)
async def _show_ranklist(self, channel, contest_id: int, handles, ranklist, vc: bool = False, delete_after: float = None, contest=None):
contest = contest or cf_common.cache2.contest_cache.get_contest(contest_id)
Copy link
Collaborator

@algmyr algmyr Aug 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big issue, but it would be neat if we could get away with only having 1 arg, rather than both contest and contest_id. Though I'm not sure what a clean option would be here.

@@ -13,13 +13,17 @@
from gi.repository import Pango, PangoCairo

import discord
import random
import random, string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit on style, prefer doing one import per line and keep imports sorted. I think the codebase is a bit inconsistent on that atm, but it's good to try to fix such issues as we go/don't make it messier.

if is_int(contest_id):
contest_id = int(contest_id)
if contest_id<0:
contest_id = -1*contest_id
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned elsewhere, not dealing with negative ints would be nicer.

indexes[i] = letters[i]
standings = clist.format_standings(statistics, index_map, indexes)
ranklist = CRanklist(contest, standings, deltas=deltas if rated else None, problems_indexes=indexes)
return ranklist
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of these functions are on the form

if cf: minor logic + dispatch to cf_common

do complex clist logic

It might make sense to have a some overarching util (maybe resource_util for lack of better words) that does the backend agnostic work+the minor logic in the pseudocode above, and dispatches to cf_util or clist_util for specific implementations of e.g. ranklist. I.e. that ranklist functions does minor logic followed by dispatch to the specific impl of ranklist.

I suppose the resource code should also move there, just leaving the clist impl details here.

@@ -5,6 +5,7 @@
from discord.ext import commands
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meooow25 or @narutse, could you look at the db related code?

@@ -0,0 +1,25 @@
from bs4 import BeautifulSoup
import requests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to make this use aiohttp like the rest of the code which makes the request async.

@@ -0,0 +1,25 @@
from bs4 import BeautifulSoup
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File comment:
If the core clist identify logic gets moved to clist_common this scraper code could be moved there.

offset+=1000
return results

class Contest(CfContest):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having Contest subclass CfContest to change one field is a bit weird. Should probably put this in a common class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants