Created attachment 176459 [details] [review]
Patch adds checking of gnc_commodity_namespace in glists

In src/base-typemaps.i check for return type gnc_commodity_namespace is added so that it is returned instead of gnc_type_monetary.

(In reply to comment #1)
> Created an attachment (id=176459) [details] [review]
> Patch adds checking of gnc_commodity_namespace in glists
> 
> In src/base-typemaps.i check for return type gnc_commodity_namespace is added
> so that it is returned instead of gnc_type_monetary.

Please have a look at this for I am a novice. For me it works but I have at least one question :

What is the difference between src/base-typemaps.i and src/optional/python-bindings/glib.i ? In the latter the same check for glist is done.

Created attachment 176461 [details] [review]
Changes to example Script to include added namespace functionality

The script now includes the possibility to print out all namespaces.

The script accidentally resides in optional/... instead of src/optional/...

(In reply to comment #4)
> The script accidentally resides in optional/... instead of src/optional/...

Oops. Now moved to correct location.

(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=176459) [details] [review] [details] [review]
> > Patch adds checking of gnc_commodity_namespace in glists
> > 
> > In src/base-typemaps.i check for return type gnc_commodity_namespace is added
> > so that it is returned instead of gnc_type_monetary.

> 
> Please have a look at this for I am a novice. For me it works but I have at
> least one question :
> 
> What is the difference between src/base-typemaps.i and
> src/optional/python-bindings/glib.i ? In the latter the same check for glist is
> done.

Since glib.i doesn't appear in Makefile.am I am assuming(!) that it's obsolete and it's functionality has been moved to base-typemaps.i.  If someone confirms this is the case I think it should be removed to avoid further confusion.

Comment on attachment 176459 [details] [review]
Patch adds checking of gnc_commodity_namespace in glists

Thanks a lot for the patch.

Indeed, src/optional/python-bindings/glib.i can be removed, a search with grep reveals it not being used anywhere, most importantly in gnucash_core.i.

The history of this was that there was an effort to move all of the the python binding functionality to shared .i files instead of ones specificic to the python bindings. Some of that did manage to take place, which is why there is a guarded section for python binding stuff in src/base-typemaps.i .

Hopefully more progress on that can happen eventually, it is the right thing to do.

I'm currious as to the status of the example scripts patch (https://bugzilla.gnome.org/attachment.cgi?id=176461), I see that the patch to src/base-typemaps.i was commited, but not the script patch?

As for the commited patch to src/base-typemaps.i, Christoph asked on list what I thought of it -- looks fine to me.

I'm tempted to add a high level wrapper for GncCommodityNamespace just like GncCommodity and GncCommodityTable. If we overrode some of these functions that return lists to return lists of these wrapper types, the example scripts would become a lot cleaner.

(In reply to comment #8)
> Indeed, src/optional/python-bindings/glib.i can be removed, a search with grep
> reveals it not being used anywhere, most importantly in gnucash_core.i.
> 
> The history of this was that there was an effort to move all of the the python
> binding functionality to shared .i files instead of ones specificic to the
> python bindings. Some of that did manage to take place, which is why there is a
> guarded section for python binding stuff in src/base-typemaps.i .
> 
> Hopefully more progress on that can happen eventually, it is the right thing to
> do.
> 
> I'm currious as to the status of the example scripts patch
> (https://bugzilla.gnome.org/attachment.cgi?id=176461), I see that the patch to
> src/base-typemaps.i was commited, but not the script patch?

The example script should be in the correct place now. The patch lacked the leading src/... and I didn't notice, so it went missing for a while.

> 
> As for the commited patch to src/base-typemaps.i, Christoph asked on list what
> I thought of it -- looks fine to me.
> 
> I'm tempted to add a high level wrapper for GncCommodityNamespace just like
> GncCommodity and GncCommodityTable. If we overrode some of these functions that
> return lists to return lists of these wrapper types, the example scripts would
> become a lot cleaner.

I think you should let yourself be tempted.

Created attachment 176543 [details] [review]
Changes to example Script to include added namespace functionality

This patch changes the example script to reflect the capability of reading all namespaces. It now also is at the correct position in the directory tree.

(In reply to comment #5)
> (In reply to comment #4)
> > The script accidentally resides in optional/... instead of src/optional/...
> 
> Oops. Now moved to correct location.

I guess you still have to delete the directory tree optional/... in svn for when I do locally and "svn update" I get it anew.

Created attachment 176564 [details] [review]
support for GncCommodityNamespace, better support for GncCommodity, and example enhancements

The other patch from this bug that enhances the example reports has to be applied first.

Apply with -p0.

Also available via
svn merge -c 738 http://svn.parit.ca/gnucash/branches/python-bindings/
or
svn diff -c 738 http://svn.parit.ca/gnucash/branches/python-bindings/

added a high level wrapper around GncCommodityNamespace and provided 
additional wrapping for GncCommodity and GncCommodityNamespace for 
functions that return individual or lists of those

updated the price db example scripts to use this new api

Thanks very much to Mark and Christoph for the patches.

This extra chain of empty directories which came about from earlier applied patches still needs to be removed:
optional/python-bindings/example_scripts/

i.e.
$ svn rm optional
(from top of tree in trunk)

(In reply to comment #12)
> Created an attachment (id=176564) [details] [review]
> support for GncCommodityNamespace, better support for GncCommodity, and example
> enhancements
Thanks very much for that. It's really interesting to see how things develop and to learn from first hand.
> 
> 
> updated the price db example scripts to use this new api

I'm curious why you changed for example

a=[]
if a:

to

if len(a)==0:

is it because it's more obvious what is checked ? I came to like the short form and it does the same job.

Regarding if len(a)==0: more obvious with very little cost for being obvious, thus fits with the philosophy:

$ python
>>> import this
The Zen of Python, by Tim Peters

Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
Complex is better than complicated.
Flat is better than nested.
Sparse is better than dense.
Readability counts.
Special cases aren't special enough to break the rules.
Although practicality beats purity.
Errors should never pass silently.
Unless explicitly silenced.
In the face of ambiguity, refuse the temptation to guess.
There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
Now is better than never.
Although never is often better than *right* now.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
Namespaces are one honking great idea -- let's do more of those!

Cool ;-) I guess I'm still stuck with one of my favourite books : obfuscated C ;-)

(In reply to comment #17)
> Cool ;-) I guess I'm still stuck with one of my favourite books : obfuscated C
> ;-)

Though another thing I came across the recent times was Donald Knuths literate programming.

GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=636808. Please update any external references or bookmarks.