Repository navigation
Refactor exercises to match canonical data #1762
Description
There are a number of exercises that fall into one of the following cases:
Function names that differ from those specified in the canonical data
armstrong-numbers uses is_armstrong instead of is_armstrong_number:
python/exercises/armstrong-numbers/example.py
Lines 1 to 2 in 222c029
| def is_armstrong(number): | |
| return sum(pow(int(d), len(str(number))) for d in str(number)) == number |
From the canonical-data:
{
"description": "Zero is an Armstrong number",
"property": "isArmstrongNumber",
"input": {
"number": 0
},
"expected": true
},
*Note: in these cases, the camelCase names used in the canonical data should be converted to snake_case names in the tests, example solution, and solution stub.
Test input format differs from canonical data
tournament passes a multiline string instead of a list of single-line strings.
python/exercises/tournament/tournament_test.py
Lines 51 to 57 in 222c029
| def test_there_can_be_more_than_one_winner(self): | |
| results = ('Allegoric Alaskans;Blithering Badgers;loss\n' | |
| 'Allegoric Alaskans;Blithering Badgers;win') | |
| table = ('Team | MP | W | D | L | P\n' | |
| 'Allegoric Alaskans | 2 | 1 | 0 | 1 | 3\n' | |
| 'Blithering Badgers | 2 | 1 | 0 | 1 | 3') | |
| self.assertEqual(tally(results), table) |
From the canonical-data:
{
"description": "There can be more than one winner",
"property": "tally",
"input": {
"rows": [
"Allegoric Alaskans;Blithering Badgers;loss",
"Allegoric Alaskans;Blithering Badgers;win"
]
},
"expected": [
"Team | MP | W | D | L | P",
"Allegoric Alaskans | 2 | 1 | 0 | 1 | 3",
"Blithering Badgers | 2 | 1 | 0 | 1 | 3"
]
},
Test output format differs from canonical data
saddle-points expects a set of tuples instead of a list of dict objects
python/exercises/saddle-points/saddle_points_test.py
Lines 16 to 18 in 222c029
| def test_identify_single_saddle_point(self): | |
| matrix = [[9, 8, 7], [5, 3, 2], [6, 6, 7]] | |
| self.assertEqual(saddle_points(matrix), set([(2, 1)])) |
From the canonical-data:
{
"description": "Can identify single saddle point",
"comments": [
"This is the README example."
],
"property": "saddlePoints",
"input": {
"matrix": [
[9, 8, 7],
[5, 3, 2],
[6, 6, 7]
]
},
"expected": [
{
"row": 2,
"column": 1
}
]
},
Test input order differs from canonical data
change uses the order target, coins instead of coins, target
python/exercises/change/change_test.py
Lines 9 to 10 in 222c029
| def test_single_coin_change(self): | |
| self.assertEqual(find_minimum_coins(25, [1, 5, 10, 25, 100]), [25]) |
From the canonical-data:
{
"description": "single coin change",
"property": "findFewestCoins",
"input": {
"coins": [1, 5, 10, 25, 100],
"target": 25
},
"expected": [25]
},
In an effort to simplify the job of the WIP test generator, it would be beneficial to reduce these inconsistencies now, so that it does not become the job of the generator PR to correct example solutions and stubs that do not match newly generated tests.
Here is the full list of exercises in which this occurs
- alphametics
- armstrong-numbers
- binary-search
- bob
- book-store
- change
- collatz-conjecture
- crypto-square
- diamond
- difference-of-squares
- dominoes
- gigasecond
- grains
- grep
- isbn-verifier
- knapsack
- leap
- list-ops
- markdown
- meetup
- minesweeper
- nth-prime
- palindrome-products
- prime-factors
- pythagorean-triplet
- raindrops
- rectangles
- roman-numerals
- saddle-points
- secret-handshake
- sieve
- spiral-matrix
- sublist
-
sum-of-multiples(foregoing updates for now) - tournament
- transpose
- triangle
- two-fer
- word-count
- wordy
- yacht
This issue may be considered resolved once all of the above issues have been updated to match the canonical properties.
Activity
armstrong-numbers: sync with problem-specifications (exercism#1762)
spencer51324312 commentedon May 9, 2019
@cmccandless Do we keep this checklist up to date?
cmccandless commentedon May 9, 2019
@spencer51324312 I attempt to, yes.
BethanyG commentedon May 19, 2019
@cmccandless - I would love to help with this, but want to make sure I do so in the "right" way. Should I just dig into whichever non-checked off exercise is next on the list and have at it - one pull per exercise changed?
cmccandless commentedon May 20, 2019
@BethanyG
Perfect!
BethanyG commentedon May 22, 2019
So I am going to call out binary-search. I really don't think that the
cannonical.json(which wants a function named find) is a good idea here. A find function could shadow Python'sstr.find().So - does the cannonical data change, does the exercise, or is this an exception? https://github.com/exercism/problem-specifications/blob/master/exercises/binary-search/canonical-data.json
BethanyG commentedon May 22, 2019
..on second thought, I will go ahead and do the changes, and if the answer is no, we can just kill the pull request.
Grociu commentedon May 30, 2019
I did a check for the exercise "two-fer", everything seems to be in order with that one, reflects the canonical data as is in my opinion.
cmccandless commentedon May 31, 2019
@Grociu regarding
two-fer, the mismatch is in the choice to usename=""as the null case rather thanname=NoneGrociu commentedon May 31, 2019
canonical-data.json for the
sum_of_multiplesexercise usessumas the main function name. As it is a built-in function name, and that function is actually used in theexample.pydo I go withsumand rewrite the example, or something else?What I did so far is
from sum_of_multiples import sum_of_multiples as suminto the test function and make the test cases test the importedsum()function. I don't know if that's up to the standard.cmccandless commentedon May 31, 2019
@Grociu You might consider storing the builtin definition of
sum()in another variable, i.e.BethanyG commentedon Jun 1, 2019
For
palindrome-productsI have a few issues / questions:The current Travis tests fail if I remove
from __future__ import divisionfromexample.py... but since we are EOL Python 2.7, we really should be removing it. Should we undertake that now, or shelve for when the EOL Python 2.7 gets rolling in earnest?Found the following as a helper method in the tests:
which effectively negates any data type returned by the code under test (provided that the data type is an iterable that can be made into a
setoffrozensets. ) While this gives wide latitude for exercise submissions, it does feel as though it's circumventing the point of the exercise (and the canonical-data), which is to return a list of factor lists. I did change the data types for the expected results, but did not remove the helper method. Should I have?Flake8complained that theexample.pysolution was "too complex", but I left it as-is. Thepalindromesfunction has two additional nested functions within it, which is a bit much. Should we be considering a re-factor?Grociu commentedon Jun 20, 2019
For
wordy: Do you want me to rewrite the tests so they are checking the "raises error with message"cases whether they raise the actual error messages ("unknown operation", "syntax error") as stated in canonical-data.json?
I've been toying with this approach a little, and it would require rewriting the example somewhat.
cmccandless commentedon Jun 20, 2019
Nope; the decision was made to not require students to use exact error message, only to have a message present. It's up to mentors then to comment on whether the exception messages match the case.
Grociu commentedon Jun 20, 2019
OK, thanks. Can you point me towards the reason this is not marked as done? I see an inconsistency in declaring strings
'text'vs"text"in the test function, and twice the indentation can be a little better. I have these fixed in my local branch, but I feel like I'm missing something.cmccandless commentedon Jun 20, 2019
I picked up on most of those via a script; it looks like that one might've been a false positive. I've checked it off.
cmccandless commentedon Jul 1, 2019
All items on the above list have been addressed, so I'm closing this issue.