Repository: kmyk/online-judge-tools
Pull request #582 (2019-10-26 02:26:55)

[breaking minor API] replace get_problem of submissions with download_problem
Currently, no contests support `get_problem`. So we should use `download_problem` rather than `get_problem`.

And I found `get_problem` can't be supported for most contest. I investigated AtCoder, yukicoder and Codeforces.
Once we unsupport `get_problem`. If we need it in the future, we can re-implement it.

fukatani: Something is about wrong about sphinx in Python3.5.2. 
fukatani: `sphinx-autodoc-typehints 1.9` was released yesterday and it caused build error in Python3.5.2.
I'm asking question to `sphinx-autodoc-typehints` maintainer team, whether this behavior is bug or as designed.

https://github.com/agronholm/sphinx-autodoc-typehints/issues/113
kmyk: @fukatani AtCoder と `sphinx-autodoc-typehints` の問題は別のプルリクに分離してほしいです
fukatani: すいません、邪道と思いつつ#582でAtCoder と sphinx-autodoc-typehints の解決コミットをマージしてしまいました。

このPRではもはや扱う必要がないため、コミットを抜いてリベースします。
fukatani: rebaseしました。
kmyk: @fukatani sphinx の warning のやつも別のプルリクにしてほしいです
kmyk: Indeed, `get_problem` from submissions cannot be implemented at most contest sites.
But also, I would like to be more sensitive for what functions do network access or don't. Therefore `download_problem` (i.e. allowing network access, contrary to `get_problem`) is not easy one too...
fukatani: OK, I simply removed `get_problem`.
kmyk: mypy で落ちちゃってるぽいので修正をお願いします。

``` console
$ mypy onlinejudge setup.py tests
onlinejudge/_implementation/utils.py:373: error: Signature of "download_problem" incompatible with supertype "Submission"
```
kmyk: プルリクの内容はひとつに絞ってください。このプルリクに含まれていたコミットの多く (バグ修正、リファクタリング) は「`get_problem` をやめて `download_problem` にする」というこのプルリクの内容とまったく無関係のはずです。
「ついでの改善」はプルリクを大きくするアンチパターンです。レビューを長びかせる、コンフリクトしやすくなる、元々のプルリクのマージが遅れたり拒否されたときにその改善も取り込まれない、などの悪影響があります。

まとめた方が楽な場合もあるので常にだめとは限りませんが、たとえば今回は後方互換性を崩す (つまり、すでにその機能を使っていたユーザのコードを動かなくする) ような影響度の大きいプルリクであり、焦点を絞ったプルリクであってほしいです。

参考:
- https://qiita.com/s2re/items/e6a1e94d5907028f0a11#%E3%81%A4%E3%81%84%E3%81%A7%E3%81%AE%E4%BF%AE%E6%AD%A3%E3%82%92%E5%90%AB%E3%82%81%E3%82%8B%E3%81%AA
- https://khigashigashi.hatenablog.com/entry/2018/03/09/020359
fukatani: 失礼しました。これで残りはget_problemまわりの差分だけなはずです。

Patch:
diff --git a/onlinejudge/_implementation/utils.py b/onlinejudge/_implementation/utils.py
index 38afda8..4c0c425 100644
--- a/onlinejudge/_implementation/utils.py
+++ b/onlinejudge/_implementation/utils.py
@@ -370,8 +370,8 @@ class DummySubmission(Submission):
     def get_url(self) -> str:
         return self.url
 
-    def get_problem(self) -> Problem:
-        return self.problem
+    def download_problem(self, *, session: Optional[requests.Session] = None) -> Problem:
+        raise NotImplementedError
 
     def get_service(self) -> Service:
         raise NotImplementedError
diff --git a/onlinejudge/dispatch.py b/onlinejudge/dispatch.py
index ed091ac..1852e99 100644
--- a/onlinejudge/dispatch.py
+++ b/onlinejudge/dispatch.py
@@ -54,9 +54,6 @@ def problem_from_url(url: str) -> Optional[Problem]:
         if problem is not None:
             log.status('problem recognized: %s: %s', str(problem), url)
             return problem
-    submission = submission_from_url(url)
-    if submission is not None:
-        return submission.get_problem()
     log.failure('unknown problem: %s', url)
     return None
 
diff --git a/onlinejudge/service/atcoder.py b/onlinejudge/service/atcoder.py
index 7b7bcd7..86f5494 100644
--- a/onlinejudge/service/atcoder.py
+++ b/onlinejudge/service/atcoder.py
@@ -1099,13 +1099,6 @@ class AtCoderSubmission(onlinejudge.type.Submission):
         problem_id = self.download_data(session=session).problem_id
         return AtCoderProblem(contest_id=self.contest_id, problem_id=problem_id)
 
-    def get_problem(self) -> AtCoderProblem:
-        """
-        :raises Exception:
-        :note: There is no way to reconstruct problem_id without networking
-        """
-        raise Exception
-
     def download_data(self, *, session: Optional[requests.Session] = None) -> AtCoderSubmissionDetailedData:
         """
         :note: `Exec Time` is undefined when the status is `RE` or `TLE`
diff --git a/onlinejudge/type.py b/onlinejudge/type.py
index b2f9da0..c55ca47 100644
--- a/onlinejudge/type.py
+++ b/onlinejudge/type.py
@@ -309,14 +309,6 @@ class SubmissionData(DownloadedData):
     def submission(self) -> 'Submission':
         raise NotImplementedError
 
-    @property
-    def problem(self) -> Problem:
-        return self.submission.get_problem()
-
-    @property
-    def contest(self) -> Contest:
-        return self.submission.get_contest()
-
     @property
     def service(self) -> Service:
         return self.submission.get_service()
@@ -343,15 +335,15 @@ class Submission(ABC):
         raise NotImplementedError
 
     @abstractmethod
-    def get_problem(self) -> Problem:
+    def download_problem(self, *, session: Optional[requests.Session] = None) -> Problem:
         raise NotImplementedError
 
-    def get_contest(self) -> Contest:
-        return self.get_problem().get_contest()
+    def download_contest(self) -> Contest:
+        return self.download_problem().get_contest()
 
     @abstractmethod
     def get_service(self) -> Service:
-        return self.get_problem().get_service()
+        raise NotImplementedError
 
     def __repr__(self) -> str:
         return '{}.from_url({})'.format(self.__class__.__name__, repr(self.get_url()))


Test patch:
diff --git a/tests/dispatch.py b/tests/dispatch.py
index bec074f..6a7553b 100644
--- a/tests/dispatch.py
+++ b/tests/dispatch.py
@@ -17,8 +17,6 @@ class DispatchAtCoderTest(unittest.TestCase):
         submission = dispatch.submission_from_url('https://atcoder.jp/contests/agc039/submissions/7874055')
         self.assertIsInstance(submission, service.atcoder.AtCoderSubmission)
         self.assertIsInstance(submission.get_service(), service.atcoder.AtCoderService)
-        with self.assertRaises(Exception):
-            submission.get_problem()
         problem = submission.download_problem()
         self.assertEqual(problem.contest_id, "agc039")
         self.assertEqual(problem.problem_id, "agc039_b")
@@ -59,6 +57,7 @@ class InvalidDispatchTest(unittest.TestCase):
     def test_problem_from_url(self):
         self.assertIsNone(dispatch.problem_from_url('https://atcoder.jp/contests/agc039'))
         self.assertIsNone(dispatch.problem_from_url("https://yukicoder.me/contests/241/"))
+        self.assertIsNone(dispatch.problem_from_url('https://atcoder.jp/contests/abc143/submissions/8059168'))
 
     def test_submission_from_url(self):
         self.assertIsNone(dispatch.submission_from_url('https://atcoder.jp/contests/agc039'))
diff --git a/tests/type.py b/tests/type.py
index 3098a59..013df1a 100644
--- a/tests/type.py
+++ b/tests/type.py
@@ -1,6 +1,7 @@
 import unittest
 
 from onlinejudge.dispatch import problem_from_url, service_from_url, submission_from_url
+from onlinejudge.service import atcoder
 from onlinejudge.type import Problem, Service, Submission
 
 
@@ -22,3 +23,9 @@ class TypeTest(unittest.TestCase):
     def test_submission_eq(self):
         self.assertEqual(submission_from_url('https://atcoder.jp/contests/abc143/submissions/8264863'), submission_from_url('https://atcoder.jp/contests/abc143/submissions/8264863'))
         self.assertNotEqual(submission_from_url('https://atcoder.jp/contests/abc143/submissions/8264863'), submission_from_url('https://atcoder.jp/contests/abc143/submissions/8264897'))
+
+    def test_download_contest(self):
+        submission = submission_from_url('https://atcoder.jp/contests/abc143/submissions/8059104')
+        contest = submission.download_contest()
+        self.assertIsInstance(contest, atcoder.AtCoderContest)
+        self.assertEqual(contest.contest_id, 'abc143')