Skip to content

Spruce up deterministic_cache, ecosystem and eigen #1197

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 22, 2018
Merged

Spruce up deterministic_cache, ecosystem and eigen #1197

merged 1 commit into from
Aug 22, 2018

Conversation

langner
Copy link
Member

@langner langner commented Aug 20, 2018

Next batch for #347

  • Minor improvements to docstrings and unit tests for ecosystem
  • Simplified unit tests for deterministic_cache a little bit more
  • Minor edits to eigen module, and made several function internal

@langner langner changed the title Sprice up deterministic_cache, ecosystem and eigen Spruce up deterministic_cache, ecosystem and eigen Aug 20, 2018
meatballs
meatballs requested changes Aug 20, 2018
View reviewed changes
Copy link
Member

@meatballs meatballs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of typos from me. Otherwise, looks great! Thank you.

axelrod/ecosystem.py Outdated

An ecosystem runs in the context of a previous tournament, and takes the
results as input. That means no matches are run by the ecosystem, and a
tournament needs to happen before it is create. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: create should be created

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

axelrod/ecosystem.py Outdated
tournament = axelrod.Tournament(players=players)
results = tournament.play()
ecosystem = axelrod.Ecosystem(results)
acosystem.reproduce(100)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: acosystem

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@drvinceknight
Copy link
Member

drvinceknight commented Aug 20, 2018 •
edited
Loading

No real comment from me, CI is failing on a test that seem unrelated:

FAIL: test_returns_foil_inspection_strategy_of_opponent (axelrod.tests.strategies.test_geller.TestGeller)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/Axelrod-Python/Axelrod/axelrod/tests/strategies/test_geller.py", line 65, in test_returns_foil_inspection_strategy_of_opponent
    self.versus_test(axelrod.MindReader(), expected_actions=[(D, D), (D, C), (D, D)], seed=seed)
  File "/home/travis/build/Axelrod-Python/Axelrod/axelrod/tests/strategies/test_player.py", line 521, in versus_test
    self.assertEqual(match.play(), expected_actions)
AssertionError: Lists differ: [(D, D), (D, D), (D, D)] != [(D, D), (D, C), (D, D)]
First differing element 1:
(D, D)
(D, C)
- [(D, D), (D, D), (D, D)]
?              ^
+ [(D, D), (D, C), (D, D)]
?              ^

Perhaps the seed is being offset somewhere?

EDIT: The same test has failed on #1198 (4919dd2) but there only on Appveyor (it passed on travis). I see this looks like it might be related to f474f2e

@drvinceknight drvinceknight mentioned this pull request Aug 20, 2018
Merged
marcharper
marcharper reviewed Aug 21, 2018
View reviewed changes
axelrod/ecosystem.py Outdated

def __init__(self, results: ResultSet,
fitness: Callable[[float], float] = None,
population: List[int] = None) -> None:
"""Create a new ecosystem.

Paramters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: Parameters

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@drvinceknight drvinceknight mentioned this pull request Aug 21, 2018
Closed
@drvinceknight
Copy link
Member

Appveyor is failing because of #1200, travis is failing because of #1202, I believe #1201 fixes both of those.

@drvinceknight
Copy link
Member

If you could pull-rebase/merge on to master @langner those two failures should now be fixed 👍

@langner
Spruce up deterministic_cache, ecosystem and eigen
1c088ef 
* Minor improvements to docstrings and unit tests for ecosystem
* Simplified unit tests for deterministic_cache a little bit more
* Minor edits to eigen module, and made several function internal
@langner
Copy link
Member Author

langner commented Aug 22, 2018

Thanks for fixing the seed thing, I wouldn't have figured it out that fast by myself! This should now be good to go.

@drvinceknight
Copy link
Member

No problem at all :)

This all looks good to me now, merge when you're happy @meatballs 👍

@meatballs meatballs merged commit 695a3e3 into Axelrod-Python:master Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcharper marcharper marcharper approved these changes

@meatballs meatballs meatballs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@langner @drvinceknight @marcharper @meatballs