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

pyplot: Fix exception in _backend_selection during import [backport to 1.4.x] #3541

Closed
wants to merge 1 commit into from

Conversation

wmanley
Copy link
Contributor

@wmanley wmanley commented Sep 19, 2014

if the new style introspection GObject python bindings are in use. With pygobject >= 3.13.4 the following:

from gi.repository import GObject
from matplotlib import pyplot

causes an exception to be raised:

AttributeError: When using gi.repository you must not import static modules
like "gobject". Please change all occurrences of "import gobject" to "from
gi.repository import GObject". See:
https://bugzilla.gnome.org/show_bug.cgi?id=709183

It is not valid to use both non-introspection based and introspection based PyGObject in the same process. Backend probing will import gobject (i.e. the non-introspection bindings) if it sees that the 'gtk' module is loaded.
Unfortunately it wouldn't check if this was the pygi or old-style gtk module. This commit adds this check avoiding the exception.

This check was added to PyGObject in d704033

@wmanley
Copy link
Contributor Author

wmanley commented Sep 19, 2014

Fixes #2901

@@ -91,8 +91,9 @@ def _backend_selection():
if not PyQt5.QtWidgets.qApp.startingUp():
# The mainloop is running.
rcParams['backend'] = 'qt5Agg'
elif 'gtk' in sys.modules and not backend in ('GTK', 'GTKAgg',
'GTKCairo'):
elif 'gtk' in sys.modules \
Copy link
Member

Choose a reason for hiding this comment

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

Please use () line continuation, not \.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks.

@tacaswell tacaswell added this to the v1.4.x milestone Sep 19, 2014
@tacaswell tacaswell changed the title pyplot: Fix exception in _backend_selection during import pyplot: Fix exception in _backend_selection during import [backport to 1.4.x] Sep 19, 2014
@tacaswell
Copy link
Member

cc @fariza as a gtk expert.

I seem to recall that we have some of the new-style imports someplace in the code. I may be out-of-scope for this PR, but I think if gi.repository is in the path, it should fall back to which ever backend uses the new-style import.

@wmanley
Copy link
Contributor Author

wmanley commented Sep 19, 2014

Well there's Gtk3Agg and GTK3Cairo, so I guess the equivalent would be:

    elif ('gtk' in sys.modules
            and 'gi.repository.GLib' in sys.modules
            and backend not in ('GTK3Agg', 'GTK3Cairo')):
        from gi.repository import GLib
        if GLib.MainLoop().is_running():
            if is_agg_backend:
                rcParams['backend'] = 'GTK3Agg'
            else:
                rcParams['backend'] = 'GTK3Cairo'

but I confess I don't understand the original code. My reading of gobject.MainLoop().is_running() is that it creates a new gobject.MainLoop and then checks if it's running, which it can't be because we just created it. i.e. by my reading this should always return False.

I wrote a little script to test this:

from gi.repository import GLib
m = GLib.MainLoop()
print "Initially running:", m.is_running()

def probe():
    print "New loop:", GLib.MainLoop().is_running()
    print "Old loop:", m.is_running()
    m.quit()

GLib.timeout_add(1, probe)
m.run()

and the Gtk2 style bindings equivalent:

import gobject
m = gobject.MainLoop()
print "Initially running:", m.is_running()

def probe():
    print "New loop:", gobject.MainLoop().is_running()
    print "Old loop:", m.is_running()
    m.quit()

gobject.timeout_add(1, probe)
m.run()

They both give the same result:

Initially running: False
New loop: False
Old loop: True

I agree that this these larger issues should be out-of-scope for this pull request. For one thing I imagine you wouldn't want to backport a patch that may change the selection of the backend to 1.4.0.

if the new style introspection GObject python bindings are in use.  With
pygobject >= 3.13.4 the following:

    from gi.repository import GObject
    from matplotlib import pyplot

causes an exception to be raised:

> AttributeError: When using gi.repository you must not import static modules
> like "gobject". Please change all occurrences of "import gobject" to "from
> gi.repository import GObject". See:
> https://bugzilla.gnome.org/show_bug.cgi?id=709183

It is not valid to use both non-introspection based and introspection based
PyGObject in the same process.  Backend probing will `import gobject` (i.e. the
non-introspection bindings) if it sees that the 'gtk' module is loaded.
Unfortunately it wouldn't check if this was the pygi or old-style gtk module.
This commit adds this check avoiding the exception.

This check was added to PyGObject in [d704033][1]

[1]: https://git.gnome.org/browse/pygobject/commit/?id=d704033
@tacaswell
Copy link
Member

I don't really understand this code either, I was hoping you did!

@fariza
Copy link
Member

fariza commented Sep 20, 2014

Sorry to come late to the party.
I haven't deal with mainloop in a long time. Even less in GTK3.
As soon as I get some time I will try to check this.

@wmanley
Copy link
Contributor Author

wmanley commented Sep 25, 2014

Would it be ok to have this patch merged? It might not implement the full-solution that we'd like, but it does fix the crash and is unlikely to have any undesirable side-effects.

@tacaswell tacaswell modified the milestones: v1.4.1, v1.4.x Sep 25, 2014
@pelson
Copy link
Member

pelson commented Sep 25, 2014

Would it be ok to have this patch merged? It might not implement the full-solution that we'd like, but it does fix the crash and is unlikely to have any undesirable side-effects.

The error message is pretty clear, so it seems reasonable to me. Any objection if I cherry-pick this to v1.4.x @tacaswell?

@tacaswell
Copy link
Member

No problem with cherry-picking it back from me.

@fariza
Copy link
Member

fariza commented Sep 25, 2014

Sorry.

I didn't have time to check it. I'm in the middle of some Angular mess
On 25 Sep 2014 14:15, "Thomas A Caswell" notifications@github.com wrote:

No problem with cherry-picking it back from me.


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

@pelson
Copy link
Member

pelson commented Sep 26, 2014

Merged with commit 236355c into v1.4.x. f9feefe...236355c

@pelson pelson closed this Sep 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants