search

RJT1990 / pyflux

Open source time series library for Python
BSD 3-Clause "New" or "Revised" License
2.11k stars 240 forks source link

Import matplotlib only if required to avoid dependency issues with Tkinter #51

Closed tmak closed 7 years ago

tmak commented 7 years ago

Right now it's not possible to use pyflux in a python environment which is not configured for Tk, which is the case for most server environments:

Traceback (most recent call last):
  File "...", line 1, in <module>
    import pyflux as pf
  File ".../python/lib/python2.7/site-packages/pyflux/__init__.py", line 5, in <module>
    from .arma import *
  File ".../python/lib/python2.7/site-packages/pyflux/arma/__init__.py", line 1, in <module>
    from .arma import ARIMA
  File ".../python/lib/python2.7/site-packages/pyflux/arma/arma.py", line 8, in <module>
    import matplotlib.pyplot as plt
  File ".../python/lib/python2.7/site-packages/matplotlib/pyplot.py", line 114, in <module>
    _backend_mod, new_figure_manager, draw_if_interactive, _show = pylab_setup()
  File ".../python/lib/python2.7/site-packages/matplotlib/backends/__init__.py", line 32, in pylab_setup
    globals(),locals(),[backend_name],0)
  File ".../python/lib/python2.7/site-packages/matplotlib/backends/backend_tkagg.py", line 6, in <module>
    from matplotlib.externals.six.moves import tkinter as Tk
  File ".../python/lib/python2.7/site-packages/matplotlib/externals/six.py", line 199, in load_module
    mod = mod._resolve()
  File ".../python/lib/python2.7/site-packages/matplotlib/externals/six.py", line 113, in _resolve
    return _import_module(self.mod)
  File ".../python/lib/python2.7/site-packages/matplotlib/externals/six.py", line 80, in _import_module
    __import__(name)
  File ".../python/lib/python2.7/lib-tk/Tkinter.py", line 39, in <module>
      import _tkinter # If this fails your Python may not be configured for Tk
ImportError: No module named _tkinter

This PR moves all matplotlib imports from the global scope into the different plotting functions so that Tkinter is only required if you actually use the plotting capabilities of pyflux.

RJT1990 commented 7 years ago

Looks good, thanks for the PR - hadn't really considered the effect you mentioned, so very happy to accept this PR.

I have a release planned for pypi this weekend, so this will be in version 4.0.1.

Thanks again!

tmak commented 7 years ago

@RJT1990 Thanks for the quick merge! Any update on releasing 4.0.1? We're keen to test it in our environment.

RJT1990 commented 7 years ago

I've got quite a lot of big changes in the next release, but expect to get it out in the next few days once I've written the documentation.

RJT1990 commented 7 years ago

Should work in latest version. Apologies for delay on getting this in - let me know if it isn't working for you

jorjacman commented 7 years ago

@RJT1990 Great work on the new release!

I noticed you reintroduced matplotlib in the global scope by importing it at the top of the neural network models and also by including it in the requirements file. We will still have issues with our server environments. Is there any way we can avoid this?

RJT1990 commented 7 years ago

Ah sorry - I'm an idiot for doing that. Will put out a fix tonight

jorjacman commented 7 years ago

@RJT1990 Let me know if you'd like me to submit a PR for this fix.

RJT1990 commented 7 years ago

Hey - sure, that might be helpful actually if you have time to spare. I'm a bit tied down this month so can't guarantee movement from my side.

RJT1990 commented 7 years ago

Hey - I pushed a new version that should fix this issue. It works for my Jenkins build now.