Adding remorseful prober

[drvinceknight]

This is the remaining strategy for the list from #387. There is another strategy on that list "contrite tft" but that strategy actually needs to know when a move against it was caused by noise. I suggest (after this) we close #387 and open another issue for that strategy as it will need a bit of a tweak to the Match class (or something).

Note that for some of the tests for this strategy I'm not using the helper functions. I didn't seem able to get the required behaviour with them: I got myself a big confused so found it easier to use the verbose calls. Not averse to spending time getting the helper functions working if we are keen to...

[meatballs]

That url can be shortened to just https://books.google.co.uk/books?id=ekonDAAAQBAJ

[drvinceknight]

Good shout, I'll give this a general clean up now :)

[marcharper]

fuller more complete

[drvinceknight]

Yup.

[marcharper]

This is a side conversation but you can have a player track its own intended history and then compare to its stored history without modifying the Match class. I toyed with a transformer for this a while back, it's a natural use case. Take a look at TrackHistoryTransformer.

[marcharper]

I had a couple of comments but they are minor so feel free to merge with or without making any changes.

[drvinceknight]

This is a side conversation but you can have a player track its own intended history and then compare to its stored history without modifying the Match class. I toyed with a transformer for this a while back, it's a natural use case. Take a look at TrackHistoryTransformer.

So this would be helpful for contrite TFT right?

[drvinceknight]

I had a couple of comments but they are minor so feel free to merge with or without making any changes.

Don't merge yet: will give the test helper functions a better go.

[marcharper]

So this would be helpful for contrite TFT right?

I presume so!

[drvinceknight]

Am I missing something here:

  200         #random.seed(0)
  201         #player.history = [C]
  202         #opponent.history = [C]
  203         #self.assertEqual(player.strategy(opponent), D)  # Random defection
  204         #self.assertEqual(player.probing, True)
  205         test_responses(self, player, opponent, [C], [C], [D], random_seed=0,
  206                        attrs={'probing': False}
  207                        )

Commenting out lines 200 -> 204 and replacing with 205 -> 205 (as above) do not give the same result:

Axelrod(387) ✗: python3.5 -m unittest axelrod.tests.unit.test_prober
/Users/vince/src/Axelrod/axelrod/tests/unit/test_player.py:175: DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() instead
  argspec = inspect.getargspec(PlayerClass.__init__)
..............................................F....
======================================================================
FAIL: test_remorse (axelrod.tests.unit.test_prober.TestRemorsefulProber)
After probing, if opponent retaliates, will offer a C
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/vince/src/Axelrod/axelrod/tests/unit/test_prober.py", line 206, in test_remorse
    attrs={'probing': False}
  File "/Users/vince/src/Axelrod/axelrod/tests/unit/test_player.py", line 100, in test_responses
    test_class.assertEqual(getattr(P1, attr), value)
AssertionError: True != False

----------------------------------------------------------------------
Ran 51 tests in 0.044s

FAILED (failures=1)

Looking at the source code it certainly should be, any ideas?

[drvinceknight]

Ignore me. That was a stupid question (I was misreading).

[drvinceknight]

But this diff should give the same behaviour right?

-        random.seed(0)
-        player.history = [C]
-        opponent.history = [C]
-        self.assertEqual(player.strategy(opponent), D)  # Random defection
-        self.assertEqual(player.probing, True)
+        test_responses(self, player, opponent, [C], [C], [D], random_seed=0,
+                       attrs={'probing': True})

-        player.history = [C, D]
-        opponent.history = [C, D]
-        self.assertEqual(player.strategy(opponent), C)  # Remorse
-        self.assertEqual(player.probing, False)
+        test_responses(self, player, opponent, [C, D], [C, D], [C],
+                       attrs={'probing': False})

-        player.history = [C, D, C]
-        opponent.history = [C, D, D]
-        self.assertEqual(player.strategy(opponent), D)
-        self.assertEqual(player.probing, False)
+        test_responses(self, player, opponent, [C, D, C], [C, D, D], [D],
+                       attrs={'probing': False})

It does not: the second test does not show remorse. I think the helper functions are doing something in the background that I don't understand (and becoming less of a fan of...). Am I just missing something and getting myself confused?

[drvinceknight]

I've pushed 27a065e with the above diff. The tests fail.

[drvinceknight]

ba1cce5 reverts the commit that fails. I'll leave it there in case it's helpful and/or an issue needs to be raised (does something need to be better documented?) but I'm happy (if everyone else is) for this to be merged as is. Not using the helper functions is my preference (I think): it's verbose and explicit.

[marcharper]

Your tests are fine. I'll take a closer look at the attrs options, they've worked for several other strategies.