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

Create a BoxPlot class #88

Closed
meatballs opened this issue Mar 6, 2015 · 20 comments · Fixed by #103
Closed

Create a BoxPlot class #88

meatballs opened this issue Mar 6, 2015 · 20 comments · Fixed by #103

Comments

@meatballs
Copy link
Member

This class should handle the production of a matlibplot boxplot based on a ResultSet instance passed to its init method.

The functions to generate the boxes/whiskers, xticks, and title should be separate methods with tests for each.

The method which would return an instance of matlibplot.Figure probably cannot be tested as Travis does not have matlibplot installed.

run_tournament.py would handle the creation of .png files based on the Figure object returned from this class.

@drvinceknight
Copy link
Member

Tagging myself in as I think I can help with this one.

@meatballs
Copy link
Member Author

I've made a start on this one - branch 88 in my fork.

@drvinceknight
Copy link
Member

👍 Will try to get to it.

@drvinceknight
Copy link
Member

Also, pretty sure we can add matplotlib as a dependency on travis... Let me go check that...

@drvinceknight
Copy link
Member

Yeah looks fine:

http://docs.travis-ci.com/user/languages/python/

Here's a SO question (mainly due to a problem with python 3 so not relevant): http://stackoverflow.com/questions/1739494/travis-ci-matplotlib-dependency-and-python3

@meatballs
Copy link
Member Author

I'm hitting the matlibplot problem again - must be something I don't understand about the library.

Line 47 in run_tournament.py ought to give the same results as line 48, but doesn't.

@drvinceknight
Copy link
Member

I've got some time, so taking a look now.

@drvinceknight
Copy link
Member

Found the error somewhere, I think your numpy array is carrying out integer division... Finding where that happens now.

@drvinceknight
Copy link
Member

Think I've fixed it: sending you a PR (apologies in advance if I just confused myself).

@drvinceknight
Copy link
Member

Ok, I think I have to actually 'request that you pull', I can't seem to get github to send you a nice pull request as I did not fork your fork...

https://github.com/drvinceknight/Axelrod/tree/88

Let me know if that doesn't work :) 👍

@drvinceknight
Copy link
Member

That branch failed on travis for me by the way (but I just checked that outputs where the same, didn't run tests).

@meatballs
Copy link
Member Author

I'll have a look some time tomorrow - thanks for the help

@drvinceknight
Copy link
Member

My pleasure (if I understood the issue correctly obviously).

On Sat, Mar 7, 2015 at 9:38 PM Owen Campbell notifications@github.com
wrote:

I'll have a look some time tomorrow - thanks for the help


Reply to this email directly or view it on GitHub
https://github.com/drvinceknight/Axelrod/issues/88#issuecomment-77711428
.

@meatballs
Copy link
Member Author

Yep - that works!! (But why did that need adding to the method, when it's not required within run_tournament.py)?

I'll finish this off and submit a PR when I'm done

@drvinceknight
Copy link
Member

Yep - that works!!

Awesome!

(But why did that need adding to the method, when it's not required within run_tournament.py)?

Because at the top of run_tournament we tweak the division: from __future__ import division. I wouldn't want to do this throughout axelrod but in a script that makes use of the library I thought it was fine. When we eventually package this so that people can run all sorts of different types of tournaments I'd want them to know that 'normal' Python 2 division is still going (even though it's a pain).

Any chance of you de-numpying? No problem if not, can get to that at another time.

@meatballs
Copy link
Member Author

Agggghhhhhhh!!!! Missed that subtlety entirely!!!

OK - I'll sort out the de-numpification and the try-catch for matplotlib before I create the PR.

@drvinceknight
Copy link
Member

(By the way: the test suite you wrote, really is excellent I'm just writing a bunch of tests for #52 and it makes it really easy: thanks!)

Agggghhhhhhh!!!! Missed that subtlety entirely!!!

Hehe, yeah pretty much my first go to when debugging Python: 'integer division'?

Really no problem if the de-numpy thing doesn't happen, I expect that'll be a weekend job to go through everything at some point...

@meatballs
Copy link
Member Author

On second thoughts, I'll leave the de-numpification for a separate PR.

@drvinceknight
Copy link
Member

Yeah, completely fine: we're in transition :D

On Sun, Mar 8, 2015 at 2:55 PM Owen Campbell notifications@github.com
wrote:

On second thoughts, I'll leave the de-numpification for a separate PR.


Reply to this email directly or view it on GitHub
https://github.com/drvinceknight/Axelrod/issues/88#issuecomment-77753430
.

@meatballs
Copy link
Member Author

(By the way: the test suite you wrote, really is excellent I'm just writing a bunch of tests for #52 and it makes it really easy: thanks!)

You're welcome. I had to write it for myself in order to understand properly what most of the code was actually doing!!

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 a pull request may close this issue.

2 participants