Merge lp:~pexego/openobject-server/add_weighted_average_grouping_server into lp:openobject-server

[Omar (Pexego)]

Adds possibility of select a field as group operator to perform weighted average when grouping with four simple lines in orm.py.

example:

_columns = {
     ...
     'price_unit': fields.float('Unit price', digits=(16,2), group_operator='product_qty'),
     'product_qty': fields.float('Quantity', digits=(16,2)),
     ...
 }

[Olivier Dony (Odoo) (odo-openerp)]

I like very much the simplicity of the code, but I'm concerned about the obfuscated meaning of passing a field name in the group_operator attribute. It really feels like a hack. Could we imagine something more explicit?
Some ideas:
- support an additional group_weight='product_qty' attribute on columns, that will be used to compute the weighted mean when group_operator is sum?
- use a more verbose syntax for group_operator, for example group_operator="sum/product_qty", which is relatively easy to parse but carries a clearer meaning?

What do you think?

Side note: read_group() is becoming huge, so it may be a right time to start splitting it into smaller functions if we continue to improve it, but that might go into a separate refactoring

[Omar (Pexego) (omar7r)]

Hi Olivier,

Yes, it feels like a hack, thinking in its meaning is more attractive an additional attribute on columns but I think that with other attribute will lose its set or union, I prefer the second option "sum/product_qty" it's clear, compact and it would be easy to parse.

Maybe something like this could be enough. I didn't test it:

group_operator = group_operator.split("/")
if len(group_operator) > 1 and group_operator in self._columns:
    group_weight = '"%s"."%s"' % (self._table, group_operator[1])
    flist += "%s(%s * %s)/nullif(%s(%s),0) AS %s" % (group_operator[0],qualified_field,group_weight,group_operator[0],group_weight, f)
else:
    flist += "%s(%s) AS %s" % (group_operator[0], qualified_field, f)

Unmerged revisions

4101. By Omar (Pexego) on 2012-03-20

[ADD] Adds possibility of select a field as group operator to perform weighted average when grouping

=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2012-03-09 08:19:29 +0000
+++ openerp/osv/orm.py	2012-03-20 20:04:57 +0000
@@ -2538,7 +2538,11 @@
             if flist:
                 flist += ', '
             qualified_field = '"%s"."%s"' % (self._table, f)
-            flist += "%s(%s) AS %s" % (group_operator, qualified_field, f)
+            if group_operator in self._columns:
+                group_operator = '"%s"."%s"' % (self._table, group_operator)
+                flist += "sum(%s * %s)/nullif(sum(%s),0) AS %s" % (qualified_field,group_operator,group_operator, f)
+            else:
+                flist += "%s(%s) AS %s" % (group_operator, qualified_field, f)
 
         gb = groupby and (' GROUP BY ' + qualified_groupby_field) or ''