Bug 4823 - python-mafw: mafw.Registry lacks get_renderers() method
: python-mafw: mafw.Registry lacks get_renderers() method
Status: RESOLVED FIXED
Product: PyMaemo
General
: unspecified
: All Linux
: Medium normal (vote)
: ---
Assigned To: Anderson Lizardo
: general
:
:
:
: 4824
  Show dependency tree
 
Reported: 2009-07-22 23:20 UTC by Anderson Lizardo
Modified: 2009-07-29 11:21 UTC (History)
1 user (show)

See Also:


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description Anderson Lizardo (reporter) 2009-07-22 23:20:22 UTC
get_renderers() returns a GList*, which requires a override. This is trivial,
see other examples from python-hildon that also return GList* for reference.
Comment 1 Andrea Grandi 2009-07-24 18:47:42 UTC
(In reply to comment #0)
> get_renderers() returns a GList*, which requires a override. This is trivial,
> see other examples from python-hildon that also return GList* for reference.
> 

Could this be a possible override for mafw_registry_get_renderers()
http://pastebin.ca/1505844 ?
Comment 2 Anderson Lizardo (reporter) 2009-07-24 19:11:17 UTC
Yes ;) commit it to you branch, compile (checking for any warnings), write a
unittest for it, e.g., add a few renderers, then compare get_renderers() output
with a list of the added renderers):

self.AssertTrue(registry.get_renderers(), [r1, r2])

where r1 and r2 are renderers you previously added.
Comment 3 Andrea Grandi 2009-07-24 19:36:04 UTC
(In reply to comment #2)
> Yes ;) commit it to you branch, compile (checking for any warnings), write a
> unittest for it, e.g., add a few renderers, then compare get_renderers() output
> with a list of the added renderers):
> 
> self.AssertTrue(registry.get_renderers(), [r1, r2])
> 
> where r1 and r2 are renderers you previously added.
> 

I've tried with this unittest: http://pastebin.ca/1505896
but the problem is I cannot add any Source/Renderer/Extension, because I always
get a similar error:

TypeError: cannot create instance of abstract (non-instantiable) type
`MafwRenderer'

How can I create such istance?!
Comment 4 Andrea Grandi 2009-07-27 11:11:38 UTC
Reading the MAFW documentation, I see these are abstract classes and for this
reason I cannot create an istance of them.

I suppose the concrete classes are the MAFW plugins, right?

It looks like there's no way to create an istance of them in Python, is it
right?
I cannot find a way to load "mafw-gst-renderer" or "mafw-tracker-source"
neither from Python or from a C example.

Any suggestion on how I could test the implemented method then?
Comment 5 Anderson Lizardo (reporter) 2009-07-27 15:02:20 UTC
You can implement abstract classes from Python. One similar cases are the home
widgets plugins from python-hildondesktop:

http://wiki.maemo.org/PyMaemo/HildonDesktop

Try something like the example there. Create a subclass of the abstract one:

class MyPlugin(mafw.Renderer):
    __gtype_name__ = 'MyPlugin'

(this __gtype_name__ attribute is required)
and see if you can instantiate it:

x = MyPlugin()
Comment 6 Andrea Grandi 2009-07-27 21:11:41 UTC
(In reply to comment #5)
> You can implement abstract classes from Python. One similar cases are the home
> widgets plugins from python-hildondesktop:
> 
> http://wiki.maemo.org/PyMaemo/HildonDesktop
> 
> Try something like the example there. Create a subclass of the abstract one:
> 
> class MyPlugin(mafw.Renderer):
>     __gtype_name__ = 'MyPlugin'
> 
> (this __gtype_name__ attribute is required)
> and see if you can instantiate it:
> 
> x = MyPlugin()

I can create instances of this class, but I cannot add them to the registry:

 [sbox-FREMANTLE_X86:
~/projects/pymaemo/packages/python-mafw/branches/andrea/tests] >
run-standalone.sh python2.5 test_mafw_registry_get_renderers.py
**
mafw-registry:ERROR:mafw-registry.c:741:mafw_registry_add_extension: assertion
failed: (mafw_extension_get_uuid(extension) != NULL)
/usr/bin/run-standalone.sh: line 11:  6061 Aborted                 (core
dumped) "$@"

I think I need to have a mandatory UUID (like the documentation says), but I
don't know how to add it.
Comment 7 Anderson Lizardo (reporter) 2009-07-27 21:30:46 UTC
Check existing C examples. If it is some parameter passed as a property during
gobject construction (i.e. when calling gobject_new()), you need a similar
approach in Python. Something like (untested):

import gobject
import mafw

class MyPlugin(mafw.Renderer):
    __gtype_name__ = 'MyPlugin'

x = gobject.new(MyPlugin, some_property=some_value)

where "some_property" is the property name (replace "-" with "_", e.g.
"plugin-id" becomes "plugin_id") and "some_value" is the value.

Unfortunately, I know very little about MAFW itself, so you need to check that
from your sources :)
Comment 8 Andrea Grandi 2009-07-28 11:51:18 UTC
(In reply to comment #7)
> Check existing C examples. If it is some parameter passed as a property during
> gobject construction (i.e. when calling gobject_new()), you need a similar
> approach in Python. Something like (untested):
> 
> import gobject
> import mafw
> 
> class MyPlugin(mafw.Renderer):
>     __gtype_name__ = 'MyPlugin'
> 
> x = gobject.new(MyPlugin, some_property=some_value)
> 
> where "some_property" is the property name (replace "-" with "_", e.g.
> "plugin-id" becomes "plugin_id") and "some_value" is the value.

you mean something like this?:

GObject *mafw_iradio_source_new(void)
{
    return g_object_new(MAFW_TYPE_IRADIO_SOURCE,
                "plugin", MAFW_IRADIO_SOURCE_PLUGIN_NAME,
                "name", MAFW_IRADIO_SOURCE_NAME,
                "uuid", MAFW_IRADIO_SOURCE_UUID,
                NULL);
}

I found this in mafw-iradio-source.c and _UUID is defined in
mafw-iradio-source.h, I'll try to pass those properties to my object.
Comment 9 Andrea Grandi 2009-07-28 12:25:31 UTC
The test I created passes correctly, so I think you can merge my changes (both
in mafw.override and add tests/test_mafw_registry_get_renderers.py to the main
trunk.

I can merge those changes too if you want... I just have to add my override to
mafw.override and my own test to the main test/ folder.

Tell me what do you prefear.

I start testing the source_browsing.py example if now it works or not.
Comment 10 Anderson Lizardo (reporter) 2009-07-28 16:26:45 UTC
One comment about your test scripts (test_mafw_registry_get_renderers.py and
test_mafw_registry_get_sources.py): please fix their indentation, they are not
consistent.

For all Python scripts, we try to follow PEP 8 (of most of it):
http://www.python.org/dev/peps/pep-0008/. Most important, we use 4 spaces for
indentation. Your script seems to currently use a mix of 4/8 spaces and tabs.

Another thing: for the get_renderers.py test, I suggest renaming the method
"test_get_instance" to "test_get_renderers". Sample apply to the
*get_sources.py (rename the "test_get_instance" method to "test_get_sources").
Then consider adding both tests to the existing tests/test_mafw_registry.py,
only take care to rename the "MyPlugin" classes to different names (e.g.
MySourcePlugin and MyRendererPlugin).

After you have this fixed, I can merge the content of your branch back into
trunk. :)
Comment 11 Andrea Grandi 2009-07-28 17:41:50 UTC
(In reply to comment #10)
> One comment about your test scripts (test_mafw_registry_get_renderers.py and
> test_mafw_registry_get_sources.py): please fix their indentation, they are not
> consistent.
> 
> For all Python scripts, we try to follow PEP 8 (of most of it):
> http://www.python.org/dev/peps/pep-0008/. Most important, we use 4 spaces for
> indentation. Your script seems to currently use a mix of 4/8 spaces and tabs.

I should have corrected the indentations, please give a look (just committed to
my branch)

> Another thing: for the get_renderers.py test, I suggest renaming the method
> "test_get_instance" to "test_get_renderers". Sample apply to the
> *get_sources.py (rename the "test_get_instance" method to "test_get_sources").

changed, committed

> Then consider adding both tests to the existing tests/test_mafw_registry.py,
> only take care to rename the "MyPlugin" classes to different names (e.g.
> MySourcePlugin and MyRendererPlugin).

I've copy-pasted the two test inside test_mafw_registry.py but I don't think
it's what you wanted... right? I also see that running test_mafw_registry.py I
only get:

[sbox-FREMANTLE_X86: ~/projects/pymaemo/packages/python-mafw/branches/andrea] >
run-standalone.sh python2.5 tests/test_mafw_registry.py
.
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK

shouldn't be at least 3 tests passed?


> After you have this fixed, I can merge the content of your branch back into
> trunk. :)
>
Comment 12 Anderson Lizardo (reporter) 2009-07-28 18:07:25 UTC
That's because all 3 classes on tests/test_mafw_registry.py have the same name
:).

You need to to ONE of the following:

A) move the test_get_renderers and test_get_sources to the original
TestMafwRegistry class (that contains the test_get_instance method already).
You should then have a single class with 3 methods, plus the 2 classes that
implement the test plugins.

or:

B) rename the two classes you added.

Note that one class represent a test suite, and it can contain many methods
(AKA tests). Most of our test files have a single class with many methods (i.e.
a single test suite with many tests, so I suggest you follow A.
Comment 13 Anderson Lizardo (reporter) 2009-07-28 18:10:16 UTC
Ok, another thing I just noticed now:

you should use:

self.assertEquals(reg.get_renderers(), [x1, x2])

instead of:

self.assertTrue(reg.get_renderers(), [x1, x2])

(same for get_sources())

assertTrue() is only to check for expressions for true/false. In this case, you
want to check if the lect expression evaluates to the same value as the right
expression.
Comment 14 Andrea Grandi 2009-07-28 18:16:52 UTC
(In reply to comment #13)
> Ok, another thing I just noticed now:
> 
> you should use:
> 
> self.assertEquals(reg.get_renderers(), [x1, x2])
> 
> instead of:
> 
> self.assertTrue(reg.get_renderers(), [x1, x2])
> 
> (same for get_sources())

well... then there's something wrong with overridden methods :(

I've just modified from assertTrue to assertEquals and this is the result:

[sbox-FREMANTLE_X86: ~/projects/pymaemo/packages/python-mafw/branches/andrea] >
run-standalone.sh python2.5 tests/test_mafw_registry.py
.FF
======================================================================
FAIL: test_get_renderers (__main__.TestMafwRegistry)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_mafw_registry.py", line 30, in test_get_renderers
    self.assertEquals(reg.get_renderers(), [x1, x2])
AssertionError: [<MyRendererPlugin object at 0xb7d3a0f4 (MyRendererPlugin at
0x81e7a40)>, <MyRendererPlugin object at 0xb7d3a0cc (MyRendererPlugin at
0x81e7a20)>] != [<MyRendererPlugin object at 0xb7d3a0cc (MyRendererPlugin at
0x81e7a20)>, <MyRendererPlugin object at 0xb7d3a0f4 (MyRendererPlugin at
0x81e7a40)>]

======================================================================
FAIL: test_get_sources (__main__.TestMafwRegistry)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_mafw_registry.py", line 22, in test_get_sources
    self.assertEquals(reg.get_sources(), [x1, x2])
AssertionError: [<MySourcePlugin object at 0xb7d3a16c (MySourcePlugin at
0x81e7a80)>, <MySourcePlugin object at 0xb7d3a1bc (MySourcePlugin at
0x81e7a60)>] != [<MySourcePlugin object at 0xb7d3a1bc (MySourcePlugin at
0x81e7a60)>, <MySourcePlugin object at 0xb7d3a16c (MySourcePlugin at
0x81e7a80)>]

----------------------------------------------------------------------
Ran 3 tests in 0.022s

FAILED (failures=2)
Comment 15 Anderson Lizardo (reporter) 2009-07-28 18:28:30 UTC
Try wrapping the return list around set():

self.assertEquals(set(reg.get_renderers()), set([x1, x2]))

This is done in other tests to compare list items ignoring their order.
Comment 16 Andrea Grandi 2009-07-28 18:33:08 UTC
(In reply to comment #15)
> Try wrapping the return list around set():
> 
> self.assertEquals(set(reg.get_renderers()), set([x1, x2]))
> 
> This is done in other tests to compare list items ignoring their order.

fixed and committed :)
Comment 17 Anderson Lizardo (reporter) 2009-07-28 19:18:20 UTC
Ok, I merged the changes in the mafw.override and tests/test_mafw_registry.py
files from your branch into trunk (r1265). Some more comments:

* It seems you forgot to write a test for the list_plugins method override.
* You can delete the test_mafw_registry_get_renderers.py and
test_mafw_registry_get_sources.py files from your branch.

Marking bug as FIXED. Thanks!
Comment 18 Andrea Grandi 2009-07-29 11:21:30 UTC
> * It seems you forgot to write a test for the list_plugins method override.

I didn't forget, it was just a work-in-progress :)
I'll do it today.

> * You can delete the test_mafw_registry_get_renderers.py and
> test_mafw_registry_get_sources.py files from your branch.

removed :)