Skip to content
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

Probabilistic ending spatial tournament. #674

Merged
merged 11 commits into from
Aug 4, 2016
Merged

Probabilistic ending spatial tournament. #674

merged 11 commits into from
Aug 4, 2016

Conversation

drvinceknight
Copy link
Member

This also adds two hypothesis test decorators (for the two spatial tournaments).

def __init__(self, players, prob_end, game, repetitions, noise, edges):

player_indices = list(range(len(players)))
node_indices = sorted(set([node for edge in edges for node in edge]))
Copy link
Member

Choose a reason for hiding this comment

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

This line might be less confusing as nested for loops.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

@marcharper
Copy link
Member

Looks good overall. One general comment -- there are a lot axelrod.* statements, you might consider importing some more things.

@drvinceknight
Copy link
Member Author

Thanks @marcharper, will address today or tomorrow :) 👍

- Using sets to check that spatial player graph is connected.
- Add a comment to explain test.
- Fix docstrings
- Modify imports in property tests.
@drvinceknight
Copy link
Member Author

One general comment -- there are a lot axelrod.* statements, you might consider importing some more things.

I think these were all in property.py, I think I've changed them all but let me know if not :)

Is the idea that it's better to avoid full library imports in the library itself?

@marcharper
Copy link
Member

Is the idea that it's better to avoid full library imports in the library itself?

I just prefer shorter lines in a case like this. It can speed up loops to import a deeper reference but I don't think that's a concern here.

@drvinceknight
Copy link
Member Author

Makes sense. Thanks :)

@drvinceknight
Copy link
Member Author

defcc38 adds/improves tests for noise.

@@ -168,6 +168,32 @@ def estimated_size(self):
size = self.__len__() * (1. / self.prob_end) * self.repetitions
return size


def test_graph_is_connected(edges, players):
"""
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be better as a function returning a boolean - call it graph_is_connected and return True or False. The init methods then just need a line to raise an error if False.

@meatballs meatballs merged commit 4aa2ab1 into master Aug 4, 2016
@meatballs meatballs deleted the 655 branch August 4, 2016 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants