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

Additions to Plots #791

Merged
merged 2 commits into from
Dec 13, 2016
Merged

Additions to Plots #791

merged 2 commits into from
Dec 13, 2016

Conversation

Nikoleta-v3
Copy link
Member

@Nikoleta-v3 Nikoleta-v3 commented Dec 12, 2016

Closes #776
Passing matplotlib axes.

Giving access to the matplotlib axes of the plots
in the library. If user does not specify the axes
then it uses plt.subplots.

Some measures, such as number of players, players
etc were used in more than one plot so I initiliaze
them in the class. Also, I have not yet removed the
title argument because I am not sure if is needed
for when running our tournament and saving the plots.

Please let me know you suggestions.

Passing matplotlib axes.

Giving access to the matplotlib axes of the plots
in the library. If user does not specify the axes
then it uses `plt.subplots`.

Some measures, such as number of players, players
etc were used in more than one plot so I initiliaze
them in the class. Also, I have not yet removed the
title argument because I am not sure if is needed
for when running our tournament and saving the plots.

Please let me know you suggestions.
@Nikoleta-v3 Nikoleta-v3 changed the title Closes #776 Additions to Plots Dec 12, 2016
Addressing the error. Taken from: sphinx-doc/sphinx#3212
Copy link
Member

@drvinceknight drvinceknight left a comment

Choose a reason for hiding this comment

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

One question about the addition to travis.yml but otherwise looks good to me.

@@ -15,6 +15,7 @@ install:
- pip install -r requirements.txt
- pip install sphinx
- pip install sphinx_rtd_theme
- pip install docutils==0.12
Copy link
Member

Choose a reason for hiding this comment

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

What's this for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Travis was failing because of this error:

assert not self.context, 'len(context) = %s' % len(self.context)
AssertionError: len(context) = 1```

and i just followed the sphinx issues where they said you need to also install this.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@drvinceknight
Copy link
Member

Also, I have not yet removed the
title argument because I am not sure if is needed
for when running our tournament and saving the plots.

I suggest leaving this as it is for the sake of backwards compatibility.

@marcharper
Copy link
Member

Looks good to me, I'll wait on @drvinceknight's title question to merge, or merge with my blessing as you wish.

@drvinceknight
Copy link
Member

If you're happy I'm happy. 👍

@drvinceknight drvinceknight merged commit 15ddd34 into Axelrod-Python:master Dec 13, 2016
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.

3 participants