abelcarreras / DynaPhoPy

Phonon anharmonicity analysis from molecular dynamics
http://abelcarreras.github.io/DynaPhoPy/
MIT License
113 stars 51 forks source link

Fix for matplotlib import error for systems without TkInter installed. #3

Closed bjmorgan closed 6 years ago

bjmorgan commented 6 years ago

importing matplotlib on systems that do not have TkInter installed gives a ModuleNotFoundError (see https://github.com/matplotlib/matplotlib/issues/9017) This commit works around this issue by setting the 'agg' backend for matplotlib.

abelcarreras commented 6 years ago

This seems some kind of bug in matplotlib. I guess (hope) this will be fixed in the near future. As workaround don't you think that it is better to use matplotlibrc file to change the default backend instead of modify dynaphopy code? This way this fix will affect all python codes that makes use of matplotlib in your system at once.

bjmorgan commented 6 years ago

Using the matplotlibrc file would work, but it would then be helpful if this was documented in the installation instructions.

My thinking was, set the backend to something supported on all systems, so at least things will run. If it needs to be something different, then this can be set in code or using matplotlibrc settings.

abelcarreras commented 6 years ago

In principle If matplotlib is properly installed I assume that this issue shouldn't occur. If you think that this can be useful I will include this issue with matplotlib in a troubleshooting section in dynaphopy manual. I think this is the best solution until this issue is fixed in matplotlib side.

bjmorgan commented 6 years ago

I’m not sure what you mean by “if matplotlib is properly installed”. I see this issue on a Linux machine if matplotlib is installed using pip (as would be the case using your requirements file). If matplotlib should be installed in a particular way to be compatible, could this be mentioned in the installation instructions of the README?

bjmorgan commented 6 years ago

From the matplotlib documentation here: https://matplotlib.org/faq/usage_faq.html#what-is-a-backend setting the backend using use is discouraged, unless you really want to force that choice (I don’t think that is the case here). That suggests my proposed pull request is not a good idea. Having a note about this issue in the docs might be helpful for other users.