go-counting: Implement the exercise "go-counting" #767
Conversation
There was a problem hiding this comment.
Please add the exercise to config.json - https://github.com/exercism/python/blob/master/config.json
yunchih
force-pushed
the
pr-go-counting
branch
from
fc53246 to
249cdaa
Compare
|
Thanks for the head-up! Added |
config.json
Outdated
| "topics": [ | ||
| "parsing", | ||
| "tuples", | ||
| "optional values" |
There was a problem hiding this comment.
We've switched to snake_case naming convention for topics to increase consistency among tracks, so could you please choose them from the following list - https://github.com/exercism/problem-specifications/blob/master/TOPICS.txt
There was a problem hiding this comment.
I also propose adding classes here
config.json
Outdated
| "slug": "go-counting", | ||
| "core": false, | ||
| "unlocked_by": null, | ||
| "difficulty": 1, |
There was a problem hiding this comment.
I guess the difficulty for this one should be increased.
What do you think?
|
Updated as requested. Thanks for being patient on me 😆 |
|
Hi, I've added the version string. Anything else that needs update? |
There was a problem hiding this comment.
@yunchih I've left some comments. Please make the appropriate changes.
exercises/go-counting/README.md
Outdated
| [wikipedia](https://en.wikipedia.org/wiki/Go_%28game%29) or [Sensei's | ||
| Library](http://senseis.xmp.net/). | ||
|
|
||
| ### Submitting Exercises |
There was a problem hiding this comment.
RE #950, this should be ## Submitting Exercises
| import unittest | ||
| import gocounting | ||
|
|
||
| # Tests adapted from `problem-specifications//canonical-data.json` @ v1.1.0 |
There was a problem hiding this comment.
Minor change: please add 1 additional blank line above this (2 before, 1 after) for consistency across exercises.
|
|
||
| class GoCountingTest(unittest.TestCase): | ||
| def test_5x5_for_black(self): | ||
| board = gocounting.GoCounting(board5x5) |
There was a problem hiding this comment.
class GoCounting should not be the same name as the module gocounting. Please rename to something more intuitive, such as Board
| def test_5x5_for_black(self): | ||
| board = gocounting.GoCounting(board5x5) | ||
| stone, territory = board.territoryFor((0, 1)) | ||
| self.assertEqual(stone, board.black) |
There was a problem hiding this comment.
It is better to define constants at either the module level
BLACK = "B"
WHITE = "W"
NONE = ""
class GoCounting(object):
...or in a separate class
class Player(object):
BLACK = "B"
WHITE = "W"
NONE = ""
class GoCounting(object):
...There was a problem hiding this comment.
You'll need to make the related changes in go_counting_test.py as well.
|
@cmccandless Thanks for your review! You've been very kind. |
There was a problem hiding this comment.
I recommend running the tests yourself before pushing new commits. The easiest way to do this without heavy modifications to your code would be to temporarily change the import statement in go_counting_test.py to import from example instead of go_counting, then just run pytest. Just make sure to revert that one change before commiting.
| def test_5x5_for_black(self): | ||
| board = gocounting.GoCounting(board5x5) | ||
| stone, territory = board.territoryFor((0, 1)) | ||
| self.assertEqual(stone, board.black) |
There was a problem hiding this comment.
You'll need to make the related changes in go_counting_test.py as well.
yunchih
force-pushed
the
pr-go-counting
branch
from
9c9f867 to
8f4059c
Compare
|
@cmccandless updated as requested. |
|
Merged, thanks @yunchih! |
See #748