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

#318: add a function to get information of problems #343

Merged
merged 1 commit into from
Feb 27, 2019

Conversation

kmyk
Copy link
Member

@kmyk kmyk commented Feb 27, 2019

先にpropertyだけ置いておいた部分の中身の実装です。問題名とかがURLから直で取れるようになります

@kmyk kmyk added this to the v6.0.0 milestone Feb 27, 2019
@kmyk kmyk requested a review from fukatani February 27, 2019 06:24

def test_get_score(self):
self.assertEqual(AtCoderProblem.from_url('https://atcoder.jp/contests/future-contest-2018-final/tasks/future_contest_2018_final_a').get_score(), 50000000)
self.assertEqual(AtCoderProblem.from_url('https://atcoder.jp/contests/abc001/tasks/abc001_4').get_score(), None)
Copy link
Contributor

Choose a reason for hiding this comment

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

get_score()がいつNoneを返すのか説明を書いたほうがいいと思いますが、書くべき場所が無いですかねえ。。

Copy link
Member Author

Choose a reason for hiding this comment

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

この指摘は正しいです。
#344 で触れられてる check_with はまさにこの None を処理するためにありました。 check_with みたいな機構が必要な場合はどれもドキュメントが必要な場合であるはずなので、check_with は消して get_score は生で定義して docstring を書くという解決がよい気がしてきました。

@fukatani fukatani assigned kmyk and unassigned fukatani Feb 27, 2019
@kmyk
Copy link
Member Author

kmyk commented Feb 27, 2019

@fukatani コメントの返答に書いた形で解決をしたいです。 #343 #344 をまたぐ形なので、両方を merge するか両方を close するかのどちらかをお願いします。
そうしてもらえば、追加で修正部分のプルリクを出すか、手元で merge してひとつのプルリクにまとめなおしてから出すかのどちらかをします。

@kmyk kmyk assigned fukatani and unassigned kmyk Feb 27, 2019
@fukatani
Copy link
Contributor

@kmyk わかりました。
コメントした箇所以外はLGTMなので、一旦両方ともマージします。

@fukatani fukatani merged commit d8ec3f4 into master Feb 27, 2019
@kmyk kmyk deleted the feature/atcoder-problem-details branch March 4, 2019 12:04
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.

2 participants