Refactor exercises to match canonical data

[cmccandless]

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.

[spencer51324312]

@cmccandless Do we keep this checklist up to date?

[cmccandless]

@spencer51324312 I attempt to, yes.

[BethanyG]

@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]

@BethanyG

Should I just dig into whichever non-checked off exercise is next on the list and have at it - one pull per exercise changed?

Perfect!

[BethanyG]

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's str.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]

..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]

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]

@Grociu regarding two-fer, the mismatch is in the choice to use name="" as the null case rather than name=None

[Grociu]

canonical-data.json for the sum_of_multiples exercise uses sum as the main function name. As it is a built-in function name, and that function is actually used in the example.py do I go with sum and rewrite the example, or something else?

What I did so far is from sum_of_multiples import sum_of_multiples as sum into the test function and make the test cases test the imported sum() function. I don't know if that's up to the standard.

[cmccandless]

@Grociu You might consider storing the builtin definition of sum() in another variable, i.e.

old_sum = sum
def new_sum(nums, maximum):
    return old_sum(...)

sum = new_sum

[BethanyG]

For palindrome-products I have a few issues / questions:

1. The current Travis tests fail if I remove from __future__ import division from example.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?

2. Found the following as a helper method in the tests:

def assertFactorsEqual(self, actual, expected):
    self.assertEqual(set(map(frozenset, actual)),
                     set(map(frozenset, expected)))

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 set of frozensets. ) 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?

3. My Flake8 complained that the example.py solution was "too complex", but I left it as-is. The palindromes function has two additional nested functions within it, which is a bit much. Should we be considering a re-factor?

[Grociu]

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]

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]

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]

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]

All items on the above list have been addressed, so I'm closing this issue.