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

Automatically set a backend if no default backend is configured for matplotlib #4086

Closed
2 tasks
JEM-Mosig opened this issue Jul 23, 2019 · 10 comments · Fixed by #4923
Closed
2 tasks

Automatically set a backend if no default backend is configured for matplotlib #4086

JEM-Mosig opened this issue Jul 23, 2019 · 10 comments · Fixed by #4923
Assignees
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR

Comments

@JEM-Mosig
Copy link
Contributor

Description of Problem:
On Mac OS X, when running

rasa init --no-prompt
rasa train nlu
rasa test nlu

testing fails because of an issue with matplotlib, iff matplotlib has not been used before. In particular, on Mac OS X, matplotlib requires a different backend setting, and presently it does not find that setting by itself. The issue is explained on stack overflow.

Overview of the Solution:
It would be convenient if rasa would recognise the problem and point the user to the solution. Alternatively, rasa could even create the necessary settings for matplotlib after prompting the user.

Definition of Done:

  • Feature has been tested by deleting the matplotlib settings file on a Mac OS X device and running the code above
  • Feature has been tested by deleting the backend: TkAgg line in the matplotlibrc file (see SE answer) on a Mac OS X device and running the code above
@JEM-Mosig JEM-Mosig added the type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR label Jul 23, 2019
@JEM-Mosig JEM-Mosig self-assigned this Jul 23, 2019
@amn41
Copy link
Contributor

amn41 commented Jul 23, 2019

might also make sense to add a --no-plot option or some other way to configure not running matplotlib at all

@JEM-Mosig
Copy link
Contributor Author

JEM-Mosig commented Jul 23, 2019

@amn41 That is also related to another issue: #3549

@tmbo
Copy link
Member

tmbo commented Sep 3, 2019

I think the --no-plot flag is separate but related.

This issue should be fixed as stated in the SO post using

import matplotlib
matplotlib.use('TkAgg') # MUST BE CALLED BEFORE IMPORTING plt
import matplotlib.pyplot as plt

@tmbo tmbo added the area:rasa-oss 🎡 Anything related to the open source Rasa framework label Sep 3, 2019
@tmbo tmbo changed the title Explain what to do when matplotlib fails on MacOSX on first use Automatically set a backend if no default backend is configured for matplotlib Sep 3, 2019
@tmbo
Copy link
Member

tmbo commented Sep 3, 2019

Even better would be to check if a backend is set and only set one if none is set.

@wochinge
Copy link
Contributor

@JEM-Mosig Are you working on this?

@JEM-Mosig
Copy link
Contributor Author

Argh... its on my ToDo list, but has low priority.

@elyase
Copy link

elyase commented Oct 22, 2019

Can someone reproduce this issue on the latest matplotlib? I think the referred stackoverflow issue is not present anymore on newer matplotlib + OSX versions.

Also I am not sure doing:

import matplotlib
matplotlib.use('TkAgg')

is the best way to go about this because it would fail in headless environments (ex: docker) where tk is not installed. In fact I can't run the tests on a new ubuntu install because of this reason but I guess that's a different issue.

I would recommend not establishing a hard dependency on matplotlib with a reasoning similar to scikit-optimize/scikit-optimize#637 (comment):

There are good reasons for why scientific libraries don't usually have matplotlib as a direct dependency: it is very platform dependent and it is usually not essential for what these libraries are trying to accomplish. Why should an optimization library depend so heavily on a visualization library?

I agree that you shouldn't try to fix dependencies for the user and I would go even further: you shouldn't even install it for them.

Take pandas as example: they have the plot() method on dataframes and series that call matplotlib.pyplot.plot() but they only import it at the time of the call and matplotlib isn't even a dependency of pandas! And the reason is very simple: pandas is a data manipulation library, the visualization part is just a kind of add-on. Leaving matplotlib out by default instead of pushing it into users that don't even need it seems like a reasonable decision to make.

Update: Created a new issue for the matplotlib dependency issue here.

@wochinge
Copy link
Contributor

@elyase Does it work if you have these lines ~/.matplotlib/matplotlibrc

backend: Agg

@elyase
Copy link

elyase commented Oct 24, 2019

@wochinge May be there a confusion. I can't reproduce the current issue (in OSX) and I was asking if somebody else had the issue because I think it is fixed in recent matplotlib versions.
There is a second somewhat related issue I created here

@degiz
Copy link
Contributor

degiz commented Dec 9, 2019

After implementing the change, I've double checked the behaviour with following setups:

  1. macOs with Python 3.7.4 and Matplotlib 3.0.3, default backend set to "'MacOSX'", which successfully renders the plots
  2. docker image "python:3.6-slim" (essentially Debian, the one that Rasa derives from in Dockerfile) with Matplotlib 3.0.3, default backend is "TkAgg", which also successfully renders the plots

For a situation when default backend is not set, we will manually try to use TkAgg or Agg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants