Only import matplotlib when needed #48
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timj
approved these changes
Dec 14, 2016
|
I have not yet tried the tests with plotting enabled but will do so before merging. |
r-owen
force-pushed
the
tickets/DM-8656
branch
from
781f0c4
to
7138c3d
Compare
|
Good suggestion to test the plotting. Setting the back end to |
Some tests and one bit of library code were importing matplotlib even if plotting was not requested. This could cause needless failures (not to mention unwanted rebuilds of the font cache). I standardized the following: - Only import matplotlib when plotting is specifically requested. - Library code logs a warning and continues if importing matplotlib fails. One bit of code did that already, but printed a message to stderr instead of logging the message. - Unit tests raise an exception if importing matplotlib fails. This is reasonable because you have to modify a test to make it plot. - `import matplotlib.pyplot as plt`, as per our coding standards.
r-owen
force-pushed
the
tickets/DM-8656
branch
from
7138c3d
to
664f071
Compare
|
I also did what I could to test the plotting changes in the library code. Two of the three files apparently aren't being run by unit tests. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some tests and one bit of library code were importing matplotlib
even if plotting was not requested. This could cause needless failures
(not to mention unwanted rebuilds of the font cache).
I standardized on only importing when plotting is specifically requested,
and the library code prints an error message and continues
(whereas the unit tests fail, a safe thing to do because they
do not normally plot anyway).