-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
yacht: implement exercise #1368
Conversation
exercises/yacht/example.py
Outdated
|
||
|
||
def ns(number, dice): | ||
return number * len([n for n in dice if n == number]) |
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.
Might I suggest return sum(n for n in dice if n == number)
?
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.
Good point! That's much more Pythonic.
exercises/yacht/example.py
Outdated
return 50 if len(set(dice)) == 1 else 0 | ||
|
||
|
||
categories = { |
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.
Perhaps it would be better to use some sort of enumeration rather than strings? Example:
from collections import Counter
from functools import partial
YACHT = 0
ONES = 1
TWOS = 2
THREES = 3
FOURS = 4
FIVES = 5
SIXES = 6
FULL_HOUSE = 7
FOUR_OF_A_KIND = 8
LITTLE_STRAIGHT = 9
BIG_STRAIGHT = 10
CHOICE = 11
# Example test
import yacht
from yacht import score
class YachtTest(unittest.TestCase):
def test_yacht(self):
self.assertEqual(score([5, 5, 5, 5, 5], yacht.YACHT), 50)
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 was taking the approach widely adopted in Python data science community, e.g. in pandas DataFrame.dropna(axis=0, how='any', thresh=None, subset=None, inplace=False)
accepts strings 'any'
and 'or'
as its how
argument. But then robot-simulator came to my mind, I guess enumeration would be better and more consistent in this series of exercises.
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.
Enumeration also allows the user to use anything they wish for the constants in their implementation (strings, integers, etc) while still constraining enough for tests to be written.
"slug": "yacht", | ||
"core": false, | ||
"unlocked_by": null, | ||
"difficulty": 1, |
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 might suggest "difficulty": 2 (but I wouldn't insist on it).
Unfortunately, we do not have a good set of guidelines for determining exercise difficulty... Perhaps this is an issue worth raising.
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.
Currently there are so many exercises with difficulty 1, some of which are much more difficult than this exercise. Maybe we'll reach a consensus on this exercise after discussions in #1370.
I would rather not have a new exercise held up by difficulty level discussion... Many exercises' difficulties require reevaluation; we may as well merge this one and add it to the list.
…On Apr 9, 2018, 22:22, at 22:22, Marc Chan ***@***.***> wrote:
mRcfps commented on this pull request.
> @@ -148,6 +148,21 @@
"conditionals"
]
},
+ {
+ "uuid": "22f937e5-52a7-4956-9dde-61c985251a6b",
+ "slug": "yacht",
+ "core": false,
+ "unlocked_by": null,
+ "difficulty": 1,
Currently there are so many exercises with difficulty 1, some of which
are much more difficult than this exercise. Maybe we'll reach a
consensus on this exercise after discussions in #1370.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#1368 (comment)
|
* Use enumeration instead of plain strings to represent categories * Improve func `ns` in example solution
@cmccandless Of course we'll realize this new exercise first. And when it comes to difficulty reevaluation, I think it's a noteworthy issue for all language tracks. |
@mrcfps merged; thanks for working on this! |
pig-latin: improve documentation (exercism#1352) add "Running the tests" section to README template (exercism#1271) * add "Running the tests" section to README template * regenerate README files with new template * rewrite Running the tests section for clarity * -switch to a plaintext-readable format -add link explaining py.test vs pytest queen-attack: re-implement according to canonical data 2.1.0 (exercism#1351) * queen-attack: re-implement according to canonical data 2.1.0 * queen-attack: rewrite to pass tests v2.1.0 * queen-attack: remove redundant tests Rm `test_invalid_position_can_attack` since `Queen()` grains: update tests to v1.1.0 (exercism#1357) Fix typo and minor style issues in all READMEs (exercism#1356) * Fix typo and style in README template * Fix `shold` to `should` * Fix minor style issues in `### Common pytest options` * Add blank line after all headers * Regenerate all READMEs * Remove redundant periods in READMEs add `awaiting review` exempt label to stale.yml (exercism#1364) go-counting: adapt tests to canonical data v1.0.0 (exercism#1360) * set correct canonical data version * adapt tests and example solution to canonical data 1.0.0 * use assertSetEqual() consistently sgf-parsing: implement exercise (exercism#1359) * implement sgf-parsing * fix import statement * create entry in config.json * fix __eq__ for Python2 lens-person: forego exercise (exercism#1299) * lens-person: forego exercise `lens-person` is specific to languages with immutable data (i.e. Haskell). This concept does not exist in Python, so the exercise should be foregone. * remove bad comma Implement exercise bank-account (exercism#1260) * Implement exercise bank-account * bank-account: generate README using configlet * bank-account: fix typo and comments pascals-triangle: update tests to canonical-date v1.2.0 (exercism#1164) house: return singleton list for single verse (exercism#1354) * house: return singleton list for single verse RE exercism#1347 ([comment](exercism#1347 (comment))) * house: fix example solution to pass changed tests meetup: remove fail-safe for undefined MeetupDayException (exercism#1345) * meetup: remove fail-safe for undefined MeetupDayException Fixes exercism#1344 * meetup: define MeetupDayException in example solution * meetup: fix example solution to raise correct exception * meetup: fix flake8 violations * meetup: add exception message Curriculum (exercism#1355) * select core exercises and set exercise ordering * add missing obvious topics * make list-ops a core exercise * rational-numbers: increase difficulty * unlocked_by core exercises only (exercism/configlet#61) Ref: exercism/DEPRECATED.v2-feedback#61 (comment) alphametics: mark computationally intensive test as extra-credit (exercism#1358) * alphametics: mark computationally intensive test as extra-credit While this test is canonical, it does not technically add additional coverage. This test serves as a test for efficiency (exercism/problem-specifications#1024 (comment)) of a solution, not completeness. Furthermore, here are the run-times for this exercise from the [latest Travis build]() (at the time of this writing): | Python Version | Run-time (seconds) | | --- | --- | | 2.7 | 3.155 | | 3.3 | 2.461 | | 3.4 | 3.567 | | 3.5 | 7.270 | | 3.6 | 0.774 | Notice that the optimized example solution is only "fast" in 3.6. * alphametics: add to list of exercises allowed to skip tests in CI bank-account: update README using configlet (exercism#1366) go-counting: update README to latest description (exercism#1367) bracket-push: update tests to v1.3.0 (exercism#1369) isbn-verifier: update tests to v2.4.0 (exercism#1373) * Replace test case - "invalid character in isbn" * Add test case with only 9 digits Python "bowling" test issue. (exercism#1372) Fixes /exercism/python/exercism#1371. yacht: implement exercise (exercism#1368) * yacht: implement exercise * yacht: use enumeration of score categories * Use enumeration instead of plain strings to represent categories * Improve func `ns` in example solution crypto-square: Clarify rectangular output requirement in README (exercism#1375) scale-generator: clarify docs. (exercism#1374) * Removed most mentions of terms that were irrelevant ("diminished interval") or undefined ("accidentals"). * Removed irrelevant table * Some light reformatting
* react: re-implement according to canonical data * react: update tests to v1.2.0 * react: correct test method name * react: fix example solution py2-compatibility * `copy()` method to `[:]` * `clear()` method to reassigning an empty list * react: refactor callbacks into pure functions * Merge upstream/master into react-1257 pig-latin: improve documentation (#1352) add "Running the tests" section to README template (#1271) * add "Running the tests" section to README template * regenerate README files with new template * rewrite Running the tests section for clarity * -switch to a plaintext-readable format -add link explaining py.test vs pytest queen-attack: re-implement according to canonical data 2.1.0 (#1351) * queen-attack: re-implement according to canonical data 2.1.0 * queen-attack: rewrite to pass tests v2.1.0 * queen-attack: remove redundant tests Rm `test_invalid_position_can_attack` since `Queen()` grains: update tests to v1.1.0 (#1357) Fix typo and minor style issues in all READMEs (#1356) * Fix typo and style in README template * Fix `shold` to `should` * Fix minor style issues in `### Common pytest options` * Add blank line after all headers * Regenerate all READMEs * Remove redundant periods in READMEs add `awaiting review` exempt label to stale.yml (#1364) go-counting: adapt tests to canonical data v1.0.0 (#1360) * set correct canonical data version * adapt tests and example solution to canonical data 1.0.0 * use assertSetEqual() consistently sgf-parsing: implement exercise (#1359) * implement sgf-parsing * fix import statement * create entry in config.json * fix __eq__ for Python2 lens-person: forego exercise (#1299) * lens-person: forego exercise `lens-person` is specific to languages with immutable data (i.e. Haskell). This concept does not exist in Python, so the exercise should be foregone. * remove bad comma Implement exercise bank-account (#1260) * Implement exercise bank-account * bank-account: generate README using configlet * bank-account: fix typo and comments pascals-triangle: update tests to canonical-date v1.2.0 (#1164) house: return singleton list for single verse (#1354) * house: return singleton list for single verse RE #1347 ([comment](#1347 (comment))) * house: fix example solution to pass changed tests meetup: remove fail-safe for undefined MeetupDayException (#1345) * meetup: remove fail-safe for undefined MeetupDayException Fixes #1344 * meetup: define MeetupDayException in example solution * meetup: fix example solution to raise correct exception * meetup: fix flake8 violations * meetup: add exception message Curriculum (#1355) * select core exercises and set exercise ordering * add missing obvious topics * make list-ops a core exercise * rational-numbers: increase difficulty * unlocked_by core exercises only (exercism/configlet#61) Ref: exercism/DEPRECATED.v2-feedback#61 (comment) alphametics: mark computationally intensive test as extra-credit (#1358) * alphametics: mark computationally intensive test as extra-credit While this test is canonical, it does not technically add additional coverage. This test serves as a test for efficiency (exercism/problem-specifications#1024 (comment)) of a solution, not completeness. Furthermore, here are the run-times for this exercise from the [latest Travis build]() (at the time of this writing): | Python Version | Run-time (seconds) | | --- | --- | | 2.7 | 3.155 | | 3.3 | 2.461 | | 3.4 | 3.567 | | 3.5 | 7.270 | | 3.6 | 0.774 | Notice that the optimized example solution is only "fast" in 3.6. * alphametics: add to list of exercises allowed to skip tests in CI bank-account: update README using configlet (#1366) go-counting: update README to latest description (#1367) bracket-push: update tests to v1.3.0 (#1369) isbn-verifier: update tests to v2.4.0 (#1373) * Replace test case - "invalid character in isbn" * Add test case with only 9 digits Python "bowling" test issue. (#1372) Fixes /exercism/python/#1371. yacht: implement exercise (#1368) * yacht: implement exercise * yacht: use enumeration of score categories * Use enumeration instead of plain strings to represent categories * Improve func `ns` in example solution crypto-square: Clarify rectangular output requirement in README (#1375) scale-generator: clarify docs. (#1374) * Removed most mentions of terms that were irrelevant ("diminished interval") or undefined ("accidentals"). * Removed irrelevant table * Some light reformatting * react: update tests to v2.0.0
Implement exercise yacht. Canonical Data. Discussions are welcome!