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

Add a matplotlibrc #1194

Merged
merged 6 commits into from
Aug 17, 2018
Merged

Add a matplotlibrc #1194

merged 6 commits into from
Aug 17, 2018

Conversation

drvinceknight
Copy link
Member

@drvinceknight drvinceknight commented Aug 16, 2018

EDIT: I have also added some comments to ensure we do not reorder these
imports.

I have also added some comments to ensure we do not reorder these
imports.
@drvinceknight drvinceknight changed the title Fix the order of imports. Add a matplotlibrc Aug 16, 2018
@marcharper
Copy link
Member

How does mpl know to look for the rc file there?

@drvinceknight
Copy link
Member Author

drvinceknight commented Aug 16, 2018

How does mpl know to look for the rc file there?

It automatically picks it up from the dir the tests are run from (https://matplotlib.org/users/customizing.html#the-matplotlibrc-file), so this does not change overall behavior of the library but just ensures the correct setting for CI.

From link:

matplotlibrc in the current working directory, usually used for specific customizations that you do not want to apply elsewhere.

@marcharper
Copy link
Member

Great, thanks for the explanation.


import matplotlib
Copy link
Member

Choose a reason for hiding this comment

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

should this one occur before import axelrod?

Copy link
Member Author

Choose a reason for hiding this comment

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

Import orders don't actually matter anymore, the matplotlibrc ooverrides everything but I got myself confused there and will reorder to fit PEP8! (and a bonus rerun of CI to make sure it's still working 🤞)

@marcharper
Copy link
Member

If it works then it's fine with me. Might want to put in a comment somewhere that explains what's happening so someone doesn't move things around later, if that still matters with the rc file.

@drvinceknight
Copy link
Member Author

drvinceknight commented Aug 16, 2018

Might want to put in a comment somewhere that explains what's happening so someone doesn't move things around later, if that still matters with the rc file.

The import orders no longer matter (the rc file sets it all) but I'll add a comment to the top of the rc file so we know not to delete it :)

@meatballs meatballs merged commit 78c22f8 into master Aug 17, 2018
@meatballs meatballs deleted the fix-inconsistant-CI-again branch August 17, 2018 08:30
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.

3 participants