I can't reproduce. Gdk.Window.get_origin() is tested here: https://git.gnome.org/browse/pygobject/tree/tests/compat_test_pygtk.py#n161

Can you provide an example where it returns the wrong info?

I have some code that calls Gdk.get_origin() on a Gdk.Window object.  Pretty simple.

Somewhere in the bowels of the PFM that is pygtkcompat, the get_origin() method is calling itself recursively, not twice, but three levels deep, actually.  Each time, the invocation strips off an element from the return value, when in fact the purpose of this stub is to return just the x,y coordinates -- a 2-tuple.  My patch always returns the final two elements -- the intended pair -- rather than the existing code which returns "all but the first", which shrinks the return value each time.

I don't know why your test case doesn't end up reproducing this recursion, this inability doesn't mean it's not a bug.  Perhaps it's some Windows API version-specific difference, but it's definitely a problem.

The fix I proposed works fine, and is easily explainable conceptually.  Why not just implement it?  It would be nice to not have to apply that
patch to every PyGI installation I do, which is necessary now.  Thanks.

> I have some code that calls Gdk.get_origin() on a Gdk.Window object.  Pretty simple.

Can you provide an example?

> Somewhere in the bowels of the PFM that is pygtkcompat, the get_origin() method is calling itself recursivel

It shouldn't. How do you know it is recursing and can you provide a stack trace?

---

Please understand that I'd prefer to understand the problem first before trying to fix it.

Here's the recursive stack trace, from a pdb breakpoint at the offending line:

(Pdb) bt
  c:\python27\lib\bdb.py(400)run()
-> exec cmd in globals, locals
  <string>(1)<module>()
  o:\taipan\sabrecon\sabrecon.py(3134)<module>()
-> main() #@@@ DEBUG: pdb_main(main)
  o:\taipan\sabrecon\sabrecon.py(3128)main()
-> console.run()
  o:\taipan\sabrecon\sabrecon.py(1760)run()
-> Gtk.main()
  o:\taipan\sabrecon\sabrecon.py(2027)on_gui_console_input_key_press_event()
-> self.relocate_mouse(-8, 16)
  o:\taipan\sabrecon\sabrecon.py(1931)relocate_mouse()
-> ox, oy = Gdk.Window.get_origin(self.window.get_window())
  c:\python27\lib\site-packages\pygtkcompat\pygtkcompat.py(192)get_origin()
-> return orig_get_origin(self)[1:]  #@@@ needs bug fix: always extract final two coordinates
  c:\python27\lib\site-packages\pygtkcompat\pygtkcompat.py(192)get_origin()
-> return orig_get_origin(self)[1:]  #@@@ needs bug fix: always extract final two coordinates
> c:\python27\lib\site-packages\pygtkcompat\pygtkcompat.py(192)get_origin()
-> return orig_get_origin(self)[1:]  #@@@ needs bug fix: always extract final two coordinates

As you might expect, this is manifested in real application code, not a simple unit test.

I refer to this as PFM because whatever attribute lookup is occurring to cause this to be called recursively is untraceable at the level of pdb or any other debugger.

-------- Also...

I would be glad to try any unit test code that you provide, *that actually runs standalone*.

I'm unable to hack the code from the link you cited, to execute the unit test:
 1. Missing a 'helpers' module -- commented out
 2. No main().
 3. Using unitttest in the customary way by adding a main() with: TestGTKCompat().main() fails.
 4. Ditto instead adding a main with: TestGTKCompat().test_gdk_window()

Too much extra hacking required, I give up...

Thanks. Could it be that you call pygtkcompat.enable_gtk() multiple times?

Yes, that probably does occur.  There are a number of modules used in this application, all of which follow a canned common recipe for including the Gtk/Gdk infrastructure, something like:

    import gi
    gi.require_version('Gtk', '3.0')
    from gi.repository import Gtk
    from gi.repository import Gdk
    from gi.repository import GObject
    from gi import pygtkcompat
    pygtkcompat.enable()
    pygtkcompat.enable_gtk(version='3.0')
    import pango

Since each module may or may not be loaded with others that use/need the same infrastructure, each module includes this recipe individually.

Are you saying that the enable_gtk() invocation needs to be treated as if it were an application singleton?  If so, that is the first I've heard of that.  The application actually executes fine even if I ignore that requirement.

That said, wouldn't the fix to get_origin() in pygtkcompat I've suggested be a better implementation in light of others who might be doing the same thing?

> Are you saying that the enable_gtk() invocation needs to be treated as if it were an application singleton?

Yeah, the current code doesn't support it. We should fix this :)

---

As a workaround you can monkey patch the functions in your toplevel module so it doesn't do anything if called again:

from gi import pygtkcompat
pygtkcompat.enable()
pygtkcompat.enable_gtk(version='3.0')

pygtkcompat.enable = lambda *args, **kwargs: None
pygtkcompat.enable_gtk = lambda *args, **kwargs: None

Blecch.  Yeah, this should definitely be fixed such that it's transparent to the unwitting pygtkcompat user if it gets invoked multiple times.  That said, I'm very grateful to have a compatibility layer like this -- it would've been a huge schedule hit to port our existing PyGTK code base and utilities over to native PyGObject.

As a naive hack, I've added:

 if hasattr(sys.modules['pygtkcompat'].pygtkcompat, 'is_initialized'): 
    return
 sys.modules['pygtkcompat'].pygtkcompat.is_initialized = True

to the start of pygtkcompat.enable_gtk().  enable() doesn't need monkey-patching -- it's idempotent.

Seems to work fine, and indeed solves the recursion problem.  It also obviates the need to monkey-patch each invocation.

However, I'm still thinking you'll want to make the get_origin() fix, in case others fall into the same pitfall I did -- it's always more robust to take the fixed number of elements you know you need than to assume you know how many elements you don't need and drop those...

Created attachment 348413 [details] [review]
pygtkcompat: Allow multiple calls to enable(), enable_gtk() as long as the version matches

enable_gtk() isn't idempotent and was breaking the API when called multiple times.
This ignores the call in case the version passed is the same as for previous calls.

Review of attachment 348413 [details] [review]:

LGTM, thanks.

Thanks