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

pythonPackages.matplotlib: enable tk backend by default #51344

Merged
merged 1 commit into from Mar 29, 2019

Conversation

timokau
Copy link
Member

@timokau timokau commented Dec 1, 2018

Motivation for this change

We currently do not build mathplotlib with any backend. This can be very
confusing for users. They will try to use matplotlib and it will simply
display nothing (see #51337). We should ship at least one backend. tk
was chosen somewhat arbitrarily. The gtk backend is problematic (see
#50959 (comment))
so tkinter seems like a good choice.

Closure size before: 256190712
After: 275376184 (7.5% increase)

CC @lovek323

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@veprbl
Copy link
Member

veprbl commented Dec 2, 2018

We currently do not build mathplotlib with any backend.

You mean interactive backend. There are, of course, working non-interactive backends (e.g. agg).

We are already unconditionally providing "MacOSX" backend on macOS, so it should be fine to have a working interactive backend on Linux too. And since macOS already has one, it doesn't need tk enabled by default.

@timokau
Copy link
Member Author

timokau commented Dec 2, 2018

Yeah that is what I mean. I disabled tk for darwin and as a bonus (side-effect of doing the change in python-packages.nix it now also works with the python2 version.

@GrahamcOfBorg build python2.pkgs.matplotlib python3.pkgs.matplotlib

@FRidh
Copy link
Member

FRidh commented Dec 3, 2018

I don't think we should enable this back-end. Matplotlib as is can be used for plotting, but just not for interactive plots. If one wants interactive plots, one can choose to override matplotlib, or simply add the dependencies (e.g. tkinter) to their Python environment.

Longer-term we should get rid of the options, and simply have ways of saying how an environment should be extended when a certain option (a back-end) is desired.

@timokau
Copy link
Member Author

timokau commented Dec 3, 2018

Yes, matplotlib can be used for non-interactive plotting. But I think at least as much if not more of the use cases are interactive. With the current situation, this can be very confusing for users who expect plot.show() to actually show something. Even when installing tkinter it will not work by default, one has to explicitly enable it as a backend. When matplotlib is built with tkinter however, it will be selected by default.

Most of the closure size increase should not be unique to matplotlib for most users. I think it is very much worth it.

@veprbl
Copy link
Member

veprbl commented Dec 3, 2018

@FRidh I had the same idea, but if we really believed that matplotlib should have no windowed interactive backends by default, we would also want to disable cocoa on darwin by default...

We currently do not build mathplotlib with any backend. This can be very
confusing for users. They will try to use matplotlib and it will simply
display nothing (see NixOS#51337). We should ship at least one backend. `tk`
was chosen somewhat arbitrarily. The gtk backend is problematic (see
NixOS#50959 (comment))
so tkinter seems like a good choice.

There is already a backend provided on darwin so there is no reason to
include tk there.
@timokau
Copy link
Member Author

timokau commented Dec 15, 2018

@FRidh is this still a hard "no" from you?

@FRidh
Copy link
Member

FRidh commented Jan 3, 2019

I definitely get a Tk window when I add tkinter to an environment:

$ nix-shell -p "(python3.withPackages(ps: with ps; [ ipython matplotlib tkinter]))" -I nixpkgs=. 
these derivations will be built:
  /nix/store/0qinnl18hln46d5f2m4g256f5ayx7p4y-python3-3.7.2-env.drv
building '/nix/store/0qinnl18hln46d5f2m4g256f5ayx7p4y-python3-3.7.2-env.drv'...
created 390 symlinks in user environment

[nix-shell:~/Code/libraries/nixpkgs]$ ipython
Python 3.7.2 (default, Dec 24 2018, 03:41:55) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.1.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: %pylab                                                                                                                                                                                                                                     
Using matplotlib backend: TkAgg
Populating the interactive namespace from numpy and matplotlib

In [2]: plot(range(10), range(10))                                                                                                                                                                                                                 
Out[2]: [<matplotlib.lines.Line2D at 0x7fb3c579c780>]

In [3]: 

I didn't get it directly to work with notebook, but if there's an issue there, it's unrelated to the matplotlib build.

@timokau
Copy link
Member Author

timokau commented Jan 6, 2019

Thats apparently because %pylab does some magic. With plain python or ipython no graphical plot is shown:

$ nix-shell -p "(python3.withPackages(ps: with ps; [ ipython matplotlib tkinter]))"

[nix-shell:~]$ ipython3
Python 3.7.2 (default, Dec 24 2018, 03:41:55) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.1.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from matplotlib.pyplot import plot

In [2]: plot(range(10), range(10))
Out[2]: [<matplotlib.lines.Line2D at 0x7f054b7d4940>]

In [3]:

But when tkinter was present at compile time of matplotlib, that does work.

@schmittlauch
Copy link
Member

I just ran into this as well and can confirm that, even when having tkinter in the python environment, a pyplot.show() just does nothing.

@FRidh
Copy link
Member

FRidh commented Feb 21, 2019

Thats apparently because %pylab does some magic. With plain python or ipython no graphical plot is shown:

Turns out it actually enabled the WxAgg backend.

But when tkinter was present at compile time of matplotlib, that does work.

Indeed, so it seems this backend (just as Cocoa) actually need to be build.

Go for it, but include this observation that these two backends, contrary to the others, require building.

@infinisil
Copy link
Member

I just encountered such problems when testing osmnx for #57426. That package really wants to have a working graphical backend for matplotlib. And simply overriding with enableTk = true; doesn't work, as two other dependencies also depend on matplotlib, so you end up with something like

osmnx.override rec {
  matplotlib = super.matplotlib.override { enableTk = true; };
  descartes = super.descartes.override { inherit matplotlib; };
  geopandas = super.geopandas.override { inherit descartes; };
}

I think this PR is alright like this, the slight closure size increase is worth the reduced trouble for users. If there are no good objections, I'll merge this today.

@infinisil infinisil merged commit 06f883f into NixOS:master Mar 29, 2019
@timokau timokau deleted the sagenb-matplotlib-fix branch March 30, 2019 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants