Created attachment 110057 [details] [review]
gst-element-factory-fancy-constructor.diff

2008-05-08  Edward Hervey  <edward.hervey@collabora.co.uk>

	Patch by: Johan Dahlin  <johan at gnome dot org>
	* gst/__init__.py:
	* gst/gstelement.override:
	* testsuite/test_element.py:
	New 'fancy' constructor for gst.Element, allows creating elements in a
	more pythonic way (i.e. myelement = gst.Element("oggmux")).
	Fixes #530417

This breaks existing elements inheriting from gst.Element.

class RawVideoVarianceFilter(gst.Element):
   ...
    def __init__(self):
        gst.Element.__init__(self)

        self.sinkpad = gst.Pad(self.sink_template)
        self.sinkpad.set_chain_function(self.do_chain)
        self.sinkpad.set_event_function(self.do_sink_event)
        self.add_pad(self.sinkpad)
        ...

+ Trace 200014

We should revert the change for this release, from the looks of it.

I propose to revert this and close as NOTABUG.  Johan, you say it's strange to be forced to use factory functions, but I disagree.  Alessandro pointed out the obvious case where you are _not_ forced to use a factory to create an element.  It's clear how it suddenly becomes strange to have the factory name passed to the constructor here.

I also don't understand how the patch gets rid of the strangeness of the factory concept anyways.  Instead of gst.element_factory_make you just propose to call gst.Element instead, but it doesn't become any more pythonic that way.  It's quite the contrary because gst.Element is a class, and calling a class in python should return an instance of that class -- not some random subclass implicitely depending on a constructor argument.

will revert it on Friday if johan hasn't answered and proposed a fix for this.

2008-06-13  Edward Hervey  <edward.hervey@collabora.co.uk>

	* gst/__init__.py:
	* gst/gstelement.override:
	* testsuite/test_element.py:
	Revert 2008-05-08  Edward Hervey  <edward.hervey@collabora.co.uk>
	Re-opens #530417

Sorry, I was on vacation.

I was pretty sure that the gst.Element subclass code path was tested through a unittest. That needs to be added, to avoid breaking this again.

Now, one way of solving it is to do something like:

  def __call__(cls, *args, **kwargs):
      if cls == gst.Element:
         # new fancy constructor path
         return

      # just instantiate the object, so g_object_new() works

Fun fun fun!

I'm closing this. Maybe we can do something fancier when we move to pygi.