-
Notifications
You must be signed in to change notification settings - Fork 8
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
Defer matplotlib #15
base: master
Are you sure you want to change the base?
Defer matplotlib #15
Conversation
03b3a81
to
e0be60e
Compare
This fix looks right to me. Thank you. The only comment I have is that your Travis tests don't run test_Atl.py, which probably would have caught this bug (since you don't have travis install matplotlib as a dependency). So I'd suggest editing test_Atl.py to move the matplotlib stuff to a separate script (and fixing the Or possibly add matplotlib to the travis dependencies and keep the matplotlib stuff in that test file. But it seems like something you should include in your Travis CI. In the longer term, I'd also suggest that this test script should include more code patterns that you list in your Examples page of the documentation. But that's obviously a bigger job, so it deserves to be a separate issue. |
Any chance this could be merged? And a new version released? It's not a stopper of anything, but it wastes CPU cycles on GalSim's Travis CI runs. We install matplotlib there just to satisfy the (implicit) dependency on it by starlink. |
Sorry. I'll make a release sometime this week. |
Atl.py only has one function that uses graphics and this is the only file that imports matplotlib even if graphics are not going to be used. Deferring means that most users of pyast do not require matplotlib if they are not using plotting.
e0be60e
to
a206f73
Compare
Importing Grf pulls in matplotlib, which can cause conflicts, for instance between GTK2 and GTK3 symbols.
A subset of this patch has been committed as c9f6665 |
This is an attempt at fixing #14 but includes a couple of extra fixes.