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

Fallback to pgi #4810

Closed
wants to merge 2 commits into from
Closed

Fallback to pgi #4810

wants to merge 2 commits into from

Conversation

OceanWolf
Copy link
Contributor

Just discovered an alternative gtk bindings for python written in "pure python".

Apparently it works with matplotlib as they provide an example to show how to use it with matplotlib... https://github.com/lazka/pgi/blob/master/examples/matplotlib/matplotlib_example.py

I will move tests over from #4143 to test.

@mdboom
Copy link
Member

mdboom commented Jul 28, 2015

It's probably non-issue, since pgi just won't exist there, but there are certain environments, such as Google AppEngine, that restrict the use of ctypes as a security risk so this wouldn't work.

Still, an interesting alternative.

raise ImportError("Gtk3 backend requires pygobject to be installed.")
try:
import pgi
pgi.install_as_gi()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fun usage pattern...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, my thoughts exactly. I did it this way because pgi still seems like an alpha, i.e. with lots of features missing that python-gi has, but they claim that they have everything working that they put in examples, and as they put matplotlib in an example, I guess it all works for us :).

Alternatively I have just thought about adding a preferred_gi rcParam which defaults to gi but can get set to pgi, to decide which one exists as the fallback. That allows people to easily try pgi even if they python-gi installed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is better than import pgi secretly having side effects. I am pretty sure that function is doing fun things to the path/import hooks.

On further consideration, this actually is a reasonable pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the code exists on github, https://github.com/lazka/pgi/blob/master/pgi/__init__.py

it basically just does:

import pgi
sys.modules["gi"] = pgi

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh... let's not do it that way. I don't want to be in the position of
messing around with imports like that, because that can have unintended
side-effects for users who are unaware that matplotlib is mangling the
modules.

On Wed, Jul 29, 2015 at 9:07 AM, OceanWolf notifications@github.com wrote:

In lib/matplotlib/backends/backend_gtk3.py
#4810 (comment):

@@ -9,7 +9,12 @@ def fn_name(): return sys._getframe(1).f_code.co_name
try:
import gi
except ImportError:

  • raise ImportError("Gtk3 backend requires pygobject to be installed.")
  • try:
  •    import pgi
    
  •    pgi.install_as_gi()
    

Well the code exists on github,
https://github.com/lazka/pgi/blob/master/pgi/__init__.py

it basically just does:

import pgi
sys.modules["gi"] = pgi


Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/4810/files#r35756016.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know what you mean, I think I would prefer:

import pgi as gi

Possibly creating a gtk3_compat.py file as we do with Qt...

@tacaswell
Copy link
Member

Can you document this as an optional dependency at https://github.com/matplotlib/matplotlib/blob/master/INSTALL#L212

@OceanWolf
Copy link
Contributor Author

@mdboom Just won't exist where?

For me my motivation comes from wanting to test Gtk3 on Travis, for me to test MEP22+27 on a GUI backend, but in general, probably good to do some testing... I have tried installing PyGObject from pip, but it doesn't install, not from pip on linux (see MEP27 thread)...

For other people, I don't see why others wouldn't want to actually use it. The fact they have a "how to use with matplotlib" example shows that people do want to use it.

I have no idea about C and CTypes, I excelled with the language during the university physics courses, but whenever I see it in the real world I find it a headache to read, the code always looks so tangled and messy, distributed between various c files and h files, but meh.

@mdboom
Copy link
Member

mdboom commented Jul 28, 2015

@mdboom Just won't exist where?

On restricted Python environments, such as Google AppEngine and other server environments that remove ctypes.

@OceanWolf
Copy link
Contributor Author

@mdboom Ahh, sorry, I completely misread that. I thought you referred to this PR as a non-issue because of the limitations of pgi, and so were giving this PR a -1... my brain failed.

Anyway, tomorrow I shall move the tests to this PR, update the INSTALL guide, and think about how to improve this with an rcParam.

@OceanWolf
Copy link
Contributor Author

@lazka just a heads up to let you know we will build support directly in to MPL for pgi, and also an opportunity to tell you it looks great.

@OceanWolf
Copy link
Contributor Author

@jenshnielsen failing... my first guess would lie with the virtual machine not finding the library provided by gir1.2-gtk-3.0 which Travis installs as on the line it fails on, it has already imported gi.repository without error, and fails executing:

from gi.repository import Gtk, Gdk, GObject, GLib

How would I validate that hypothesis? Or do you think the problem lies elsewhere...

@jenshnielsen
Copy link
Member

I would setup a local ubuntu 12.04 VM and test it locally. It's much easier when you can investigate it interactively. Im guessing that it's related to the old packages in Ubuntu 12.04

@tacaswell tacaswell added this to the proposed next point release milestone Jul 29, 2015
@jenshnielsen
Copy link
Member

One of the problems is that pgi doesn't support python 2.6 so you will need to skip the tests there or perhaps test for python version before importing pgi

Im not sure what the problem is on python 2.7 and 3.x. Perhaps missing X or a to old version of gtk3?

@OceanWolf
Copy link
Contributor Author

Yes, I think it misses X11, see the docs on Gtk.init_check().

I would have hoped we could have "shown" a window without a display device actually plugged in...

@OceanWolf
Copy link
Contributor Author

Ooh, it looks like I now have the gtk side of it fixed :).

Next fail point, a question I had earlier but didn't ask... why does backend_gtk3agg import the cairo backend... I thought Agg drew raster objects and Cairo drew vector objects... so they shouldn't mix, right?

@mdboom
Copy link
Member

mdboom commented Jul 30, 2015

Cairo is required to blit (copy an image subset) to the window buffer in Gtk3. (Gtk2 had a separate API for that, but Gtk3 has a hard dependency on Cairo, so I guess they decided to not duplicate that functionality). Strictly speaking, it doesn't have to import backend_cairo -- it was just the most convenient way to get a handle to pycairo/cairoffi, since it's a little non-trivial to import. Though I wouldn't be opposed to moving the cairo importing stuff in backend_cairo to its own separate helper module.

@OceanWolf
Copy link
Contributor Author

Ahh, okay, thanks, I didn't realise that Cairo existed as a hard dependency of gtk3. Still a bit odd getting a warning about the cairo backend requires when using the Agg, but it doesn't really matter...

@OceanWolf
Copy link
Contributor Author

Just about done on this. I just have to uncomment the knownfailiureif to make Travis happy for python-2.6.

Before I do that though, should I add an rcParam, I thought about naming it backend.gi_preference. I could do it as a string, or to future proof it, a list of strings.

Also do I need to add a what's new? Or too small for that?

@tacaswell
Copy link
Member

👍 to whats_new as we are adding support for another way to get at the
gui backend

The idea of a list of strings would be an order of preference?

On Thu, Jul 30, 2015 at 12:00 PM OceanWolf notifications@github.com wrote:

Just about done on this. I just have to uncomment the knownfailiureif to
make Travis happy for python-2.6.

Before I do that though, should I add an rcParam, I thought about naming
it backend.gi_preference. I could do it as a string, or to future proof
it, a list of strings.

Also do I need to add a what's new? Or too small for that?


Reply to this email directly or view it on GitHub
#4810 (comment)
.

@OceanWolf
Copy link
Contributor Author

Yes, an order of preference, so I would set the default as ['gi', 'pgi']. I would then create matching name functions to import and a for loop that can go through calling each function and if it succeeds without error it breaks out the loop, otherwise it tries the next one in the list.

It kind of feels like overkill for now, especially as I doubt any more versions will come along, but then again, it doesn't hurt, right?

@@ -64,6 +64,17 @@ Glossary
channel. PDF was designed in part as a next-generation document
format to replace postscript

pgi
`pgi <https://pypi.python.org/pypi/pgi/>` exists as a relatively new
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Indent 4 spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, thanks the two spaces at the beginning threw off my indentation.

@OceanWolf
Copy link
Contributor Author

Okay, changes implemented, ready for another review.

@OceanWolf
Copy link
Contributor Author

So what do people think? Ready for merging (after uncommenting the line and squashing the commits)?

@fariza
Copy link
Member

fariza commented Aug 6, 2015

I haven't been able to run a simple example with pgi

import matplotlib
matplotlib.use('GTK3Cairo')
matplotlib.rcParams['backend.gi_preference'] = ['pgi']
import matplotlib.pyplot as plt
plt.plot([1,2,3])
plt.show()

In python2.7 I get tons of

BUG: PySequence_LengthSystemError: null argument to internal routine
BUG: PySequence_LengthSystemError: null argument to internal routine
...
...
...

In python3.4 I get

Segmentation fault (core dumped)

Maybe it's my fault...

@OceanWolf
Copy link
Contributor Author

@fariza Hmm, yes, I just tested on 2.7 and got a segmentation fault...

in cairo_save () from /usr/lib/x86_64-linux-gnu/libcairo.so.2

but like you that could also just occur on my system, I have a very twisted system... the problem only occurs on plt.show() which I didn't run on this system. Just tested cleanly on Travis and it passes, albeit with a cairo error...

/build/buildd/cairo-1.10.2/src/cairo.c:173: _cairo_error: Assertion `(status != CAIRO_STATUS_SUCCESS && status <= CAIRO_STATUS_LAST_STATUS)' failed

I will post an issue on the pgi github site and see what they say...

Even with this failing, I still like this PR as we test Travis, but I do have strong doubts about advertising it until pgi becomes a bit more stable. Perhaps we advertise it as "experimental".

@OceanWolf OceanWolf mentioned this pull request Sep 3, 2015
FIX: Old Ubuntu doesn't have libgirepository as a dependancy of gir1.2-gtk-3.0
@OceanWolf
Copy link
Contributor Author

Hmm, travis seems to fail with:

ERROR: Failure: AttributeError ('module' object has no attribute 'test_backend_gtk3')

Any idea why?

@fariza I have now fixed that SegFault, it occured because pgi uses cairocffi whereas pygobject uses pycairo. In the GTK3 code we have an ugly hack (using C pointer manipulation, hence ugly hack) that converts the pycairo object to a cairocffi object, naturally as we expected to have to do the conversion we didn't check first what kind of object we had, and because we used an ugly pointer-manipulation hack, it worked without error until we later came to use that code.

Right now, it seems to segfault occasionally which I assume comes from a race condition over garbage collection, pgi still exists as experimental. Turning off garbage collection seems to prevent segfaults:

import gc
gc.disable()

What do people think about turning of GC for now for the GTK tests?

gi.require_version("Gtk", "3.0")
except AttributeError:
raise ImportError(
"pygobject version too old -- it must have require_version")
Copy link
Member

@NelleV NelleV Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the minimum required version here? It will be easier for a user if the errors specify which min version is supported.

#@knownfailureif(not HAS_GTK3)
@switch_backend('GTK3Agg')
def test_fig_close():
#save the state of Gcf.figs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in that function is not strictly pep8. There are spaces missing after comments.

# throw ImportError if none works
for lib in matplotlib.rcParams['backend.gi_preference']:
try:
gi = __import__(lib, globals(), locals(), [], 0)
Copy link
Member

@NelleV NelleV Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very familiar with the import mechanisms of python. Can you explain why import lib as gi wouldn't work here and why not resorting to importlib isn't an option? My very little knowledge says that import > importlib >> __import__.
Right now, it looks like we are trying to be too smart about this.

@@ -410,6 +410,9 @@ def deprecate_axes_colorcycle(value):
validate_stringlist = _listify_validator(six.text_type)
validate_stringlist.__doc__ = 'return a list'

validate_gi_preference = _listify_validator(ValidateInStrings(
'backend.gi_preference', ['gi', 'pgi']))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backend.gi, similarly to backend.qt4.

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017
@anntzer
Copy link
Contributor

anntzer commented Dec 13, 2017

Closed by #9932.

@OceanWolf
Copy link
Contributor Author

Thanks for this @anntzer I didn't realise that you had reviewed this. Thanks for getting it in.

@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 13, 2018
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

9 participants