GNOME Bugzilla – Bug 736678
Refactor overrides import/modules
Last modified: 2015-06-15 08:47:26 UTC
Created attachment 286207 [details] [review] patch I tried to add deprecation warnings for override module attribute access but decided to clean things up first. - I've removed the Registry, because it's just another layer of overrides and not really used anywhere - Instead of a DynamicModule import will now return the original IntrospectionModule in case there are no overrides or an OverridesProxyModule in case there are, which uses __getattr__ to fall back to the IntrospectionModule - All the overrides logic is now in gi.overrides To keep things like Gedit working gi.importer.modules[namespace] will point to the final module during override import. I haven't tested Gedit, but Regress in the test suite uses the same interface. I think this makes the import process easier to understand / modify, but ymmv. For the speed: before: python -mtimeit -s'from gi.repository import Gtk' 'Gtk.Button' 1000000 loops, best of 3: 0.965 usec per loop python -mtimeit -s'from gi.repository import Gtk' 'Gtk.Button()' 10000 loops, best of 3: 25.3 usec per loop after: python -mtimeit -s'from gi.repository import Gtk' 'Gtk.Button' 10000000 loops, best of 3: 0.0462 usec per loop python -mtimeit -s'from gi.repository import Gtk' 'Gtk.Button()' 10000 loops, best of 3: 23.1 usec per loop Probably not that visible in real code.. Any feedback welcome
Review of attachment 286207 [details] [review]: This cleanup is welcome as the registry mechanism has always been confusing. I didn't look at the changes too closely but something that came up immediately was putting more stuff into overrides/__init__.py. There was an idea noted in bug 687335 about actually clearing that file out so we can use it as a namespace package (PEP-0420). I don't know the details and I haven't really thought about this part of the code base much lately but it is something to consider.
Re bug 687335, moving overrides.__init__ to let's say gi.overridesutil and adjusting one import in importer.py should be it.
Created attachment 291993 [details] [review] patch-v2 Small update to handle/ignore non-strings in __all__ like before. The Gedit overrides erroneously put a function object there.
Review of attachment 291993 [details] [review]: Looks good to me, thanks!
Thanks for the review!
That refactoring broke the Gst.Fraction overrides (in the case we want to get a value from C to python): Before the commit (4d0ab13a8461f781986accc637fada3909cfb91a): $ python3 -c "from gi.repository import Gst; Gst.init(None); print(Gst.Structure.from_string('video/x-raw,framerate=10/1')[0]['framerate'])" <Gst.Fraction 10/1> After the commit (149c31beced944c72fba6ca6e096c81c1100ea2b): $ python3 -c "from gi.repository import Gst; Gst.init(None); print(Gst.Structure.from_string('video/x-raw,framerate=10/1')[0]['framerate'])" Traceback (most recent call last):
+ Trace 234994
return self.get_value(key)
The override in question is available (registered) here: http://cgit.freedesktop.org/gstreamer/gst-python/tree/gi/overrides/gstmodule.c#n131
I'll have a look. btw. gst-python in jhbuild is build for Python 2, but the overrides fail to import under Python 2.
Created attachment 302266 [details] [review] gst-python-fix This was the problem: http://cgit.freedesktop.org/gstreamer/gst-python/tree/gi/overrides/gstmodule.c#n88 The pygobject error message was unrelated/wrong.
Review of attachment 302266 [details] [review]: LGTM. Please commit to master and the 3.16 branch
Thanks, done.
Thanks a lot :)
Hello, Would it possible to get a bugfix release with that patch in 3.16? Pitivi is unusable right now on any distro with Gnome 3.16 because of it. Thanks a lot :)
(In reply to Thibault Saunier from comment #12) > Hello, > > Would it possible to get a bugfix release with that patch in 3.16? Pitivi is > unusable right now on any distro with Gnome 3.16 because of it. > > Thanks a lot :) Possibly in the next few days (my gnome dev machine is currently broken). If anyone else wants to do the release it would be appreciated, just make sure distcheck passes on Python 2.7 and 3.
Ok, 3.16.2 has been released: https://git.gnome.org/browse/pygobject/commit/?id=66311fd
Thanks Simon :)