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

Un-numpification and introduce payoff matrix #96

Merged
merged 7 commits into from
Mar 9, 2015

Conversation

langner
Copy link
Member

@langner langner commented Mar 7, 2015

No description provided.

@drvinceknight
Copy link
Member

This 'core dependency' of numpy, unless I've missed something along the way in the PRs, it's only needed to run results right? Would someone be able to use this library without numpy? (Not to run the tournament).

Not the end of the world if I've indeed missed that.

@drvinceknight
Copy link
Member

Probably worth adding a requirements.txt file and some info about pip grabbing the dependencies to the README... (Not terribly relevant to this PR, just thinking aloud).

@langner
Copy link
Member Author

langner commented Mar 7, 2015

I have not read all of the PRs along the way, and I don't understand what you mean exactly, but you do now import NumPy in axelrod/result_set.py:
https://github.com/drvinceknight/Axelrod/blob/master/axelrod/result_set.py#L1

And since result_set is used by axelrod/tournament.py, it is pretty much impossible to run a tournament without NumPy installed at this point.

@drvinceknight
Copy link
Member

Sorry for not being clear. At one point I really wanted to keep this written in pure Python except for things like plotting obviously. Do you think this is something worth aiming for? The stuff we're using numpy for could be relatively straightforwardly done in pure python at a low extra computation cost.

What do you think? Worth sticking trying to stick to pure Python or should we just embrace using appropriate libraries where they are appropriate. Hold on this isn't a conversation for here, here's an issue: #98

With regards to this actual PR: is this ready to be pulled in or is there more coming?

@langner
Copy link
Member Author

langner commented Mar 7, 2015

There is some more coming, but feel free to pull at will since I cannot predict when I will have a moment to sit down and write.

@langner langner changed the title Stuff leading up to eco variant Un-numpification and payoff matrix Mar 7, 2015
@langner langner changed the title Un-numpification and payoff matrix Un-numpification and introduce payoff matrix Mar 7, 2015
@drvinceknight
Copy link
Member

This looks like a big piece of work: thanks! Need a hand getting the tests right or are you ok to keep working on it?

@langner
Copy link
Member Author

langner commented Mar 9, 2015

The tests now pass and I resolved merge cobflicts with master.


# Save plot with average payoff matrix with winners at top.
pmatrix_ranked = [[results.payoff_matrix[r1][r2] for r2 in results.ranking] for r1 in results.ranking]
fig, ax = plt.subplots()
Copy link
Member

Choose a reason for hiding this comment

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

Does this not need to be in the line if boxplot.matplotlib_installed: statement?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because there is a conditional continue two lines before that for the opposite case.

@drvinceknight
Copy link
Member

Love the heat maps.

@drvinceknight
Copy link
Member

Missed that: awesome.

drvinceknight added a commit that referenced this pull request Mar 9, 2015
Un-numpification and introduce payoff matrix
@drvinceknight drvinceknight merged commit f3d7f30 into Axelrod-Python:master Mar 9, 2015
marcharper pushed a commit to marcharper/Axelrod that referenced this pull request Nov 2, 2015
Un-numpification and introduce payoff matrix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants