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

Avoid top-level import of pyplot for benefit of upstream packages #9

Merged
merged 2 commits into from
May 19, 2020

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Feb 15, 2019

This is a possible fix for problems with ORviewer (vis a vis the xija import) where we want to avoid a pyplot import for tools that may just be using a non-interactive backend.

This package has no tests, so testing is not so trivial.

Functional testing

Matplotlib backend not set on import

In [1]: import matplotlib

In [2]: matplotlib.get_backend()
Out[2]: 'MacOSX'

In [3]: from Ska.Matplotlib import *

In [4]: matplotlib.get_backend()  # Default backend not changed
Out[4]: 'MacOSX'

In [5]: matplotlib.use('Agg')  # Can still set backend

In [6]: import matplotlib.pyplot

In [7]: matplotlib.use('MacOSX')  # After pyplot import cannot set backend
/Users/aldcroft/miniconda3/envs/ska3/lib/python3.6/site-packages/matplotlib/__init__.py:1405: UserWarning: 
This call to matplotlib.use() has no effect because the backend has already
been chosen; matplotlib.use() must be called *before* pylab, matplotlib.pyplot,
or matplotlib.backends is imported for the first time.

  warnings.warn(_use_error_msg)

Plot functions work

In [9]: plot_cxctime([0, 100], [0, 100])
Out[9]: 
(((matplotlib.dates.SecondLocator,
   {'bysecond': (0, 15, 30, 45)},
   '%H:%M:%S',
   matplotlib.dates.SecondLocator,
   {'bysecond': [0, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55]}),),
 <matplotlib.figure.Figure at 0x7fb0189b4d68>,
 <matplotlib.axes._subplots.AxesSubplot at 0x7fb068d05c88>)

In [10]: matplotlib.pyplot.savefig('junk.png')

In [11]: from Ska.Matplotlib.lineid_plot import *

In [12]: plot_line_ids?

In [13]: plot_line_ids([0, 10], [0, 10], [5], ['label'])
Out[13]: 
(<matplotlib.figure.Figure at 0x7fb0191212b0>,
 <matplotlib.axes._axes.Axes at 0x7fb0191210f0>)

In [14]: matplotlib.pyplot.savefig('lines.png')

In [15]: !open lines.png

In [16]: !open junk.png

image

image

@jeanconn
Copy link
Contributor

I like this (and considered it as well) but wasn't sure we wanted it for the with-proseco-4.4 release. For that, I thought a change in sparkles to switch to the non-interactive backend if already defined (or to just set to backend-independend non-interactive plotting) might be considered to have smaller scope.

@taldcroft
Copy link
Member Author

I thought the problem is not sparkles but xija. I thought it wasn't possible to change backend once it has been (really) defined, but that might be wrong. By "really", I mean matplotlib.use(backend) doesn't seem to actually do anything irreversible, but once you import matplotlib.pyplot or matplotlib.backends, it seems that the backend is locked in. Again I could be wrong.

@taldcroft
Copy link
Member Author

I've done some functional testing and re-reviewed this. I think it's OK now.

@taldcroft taldcroft requested a review from jeanconn May 12, 2020 19:49
@taldcroft taldcroft merged commit 8313e03 into master May 19, 2020
@taldcroft taldcroft deleted the lazy-import branch May 19, 2020 14:14
@javierggt javierggt mentioned this pull request Dec 7, 2020
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.

2 participants