[FIX] fields: fix _column.new() by relying on to_field_args()

The implementation was based on the ill-defined method `same_parameters()` that
compares arguments based on a heuristic.  Instead, we now create a new column
and check whether it is equivalent to `self` by comparing the arguments
returned by `to_field_args()`.  If that is the case, `self` is reused instead
of the new column.

The code refactoring also fixes the column reuse which was broken by the
introduction of the parameter `compute` in commit 9333c62.  Indeed, with that
parameter, `same_parameters()` always returned False, since old-api columns do
not have that parameter by default.  The parameter has been renamed to
`_computed_field`, and is no longer passed for creating columns.

openerp/fields.py

@@ -658,7 +658,7 @@ def to_column(self):
             self.column = fields.property(**args)
         elif self.column:
             # let the column provide a valid column for the given parameters
-            self.column = self.column.new(**args)
+            self.column = self.column.new(_computed_field=bool(self.compute), **args)
         else:
             # create a fresh new column of the right type
             self.column = getattr(fields, self.type)(**args)
@@ -677,7 +677,6 @@ def to_column(self):
     _column_groups = property(attrgetter('groups'))
     _column_change_default = property(attrgetter('change_default'))
     _column_deprecated = property(attrgetter('deprecated'))
-    _column_compute = property(lambda self: bool(self.compute))
 
     ############################################################################
     #

openerp/osv/fields.py

@@ -115,6 +115,7 @@ def __init__(self, string='unknown', required=False, readonly=False, domain=None
         """
         args0 = {
             'string': string,
+            'help': args.pop('help', None),
             'required': required,
             'readonly': readonly,
             '_domain': domain,
@@ -127,6 +128,9 @@ def __init__(self, string='unknown', required=False, readonly=False, domain=None
             'translate': translate,
             'select': select,
             'manual': manual,
+            'group_operator': args.pop('group_operator', None),
+            'groups': args.pop('groups', None),
+            'deprecated': args.pop('deprecated', None),
         }
         for key, val in args0.iteritems():
             if val:
@@ -140,30 +144,23 @@ def __init__(self, string='unknown', required=False, readonly=False, domain=None
         if not self._classic_write or self.deprecated or self.manual:
             self._prefetch = False
 
-    def new(self, **args):
-        """ return a column like `self` with the given parameters """
+    def new(self, _computed_field=False, **args):
+        """ Return a column like `self` with the given parameters; the parameter
+            `_computed_field` tells whether the corresponding field is computed.
+        """
         # memory optimization: reuse self whenever possible; you can reduce the
         # average memory usage per registry by 10 megabytes!
-        return self if self.same_parameters(args) else type(self)(**args)
-
-    def same_parameters(self, args):
-        dummy = object()
-        return all(
-            # either both are falsy, or they are equal
-            (not val1 and not val) or (val1 == val)
-            for key, val in args.iteritems()
-            for val1 in [getattr(self, key, getattr(self, '_' + key, dummy))]
-        )
+        column = type(self)(**args)
+        return self if self.to_field_args() == column.to_field_args() else column
 
     def to_field(self):
         """ convert column `self` to a new-style field """
         from openerp.fields import Field
-        return Field.by_type[self._type](**self.to_field_args())
+        return Field.by_type[self._type](column=self, **self.to_field_args())
 
     def to_field_args(self):
         """ return a dictionary with all the arguments to pass to the field """
         base_items = [
-            ('column', self),                   # field interfaces self
             ('copy', self.copy),
         ]
         truthy_items = filter(itemgetter(1), [
@@ -349,7 +346,7 @@ def __init__(self, string='unknown', digits=None, digits_compute=None, required=
         # synopsis: digits_compute(cr) ->  (precision, scale)
         self.digits_compute = digits_compute
 
-    def new(self, **args):
+    def new(self, _computed_field=False, **args):
         # float columns are database-dependent, so always recreate them
         return type(self)(**args)
 
@@ -1292,9 +1289,9 @@ def __init__(self, fnct, arg=None, fnct_inv=None, fnct_inv_arg=None, type='float
                 self._symbol_f = type_class._symbol_f
                 self._symbol_set = type_class._symbol_set
 
-    def new(self, **args):
-        if args.get('compute'):
-            # field is computed, we need an instance of the given type
+    def new(self, _computed_field=False, **args):
+        if _computed_field:
+            # field is computed, we need an instance of a non-function column
             type_class = globals()[self._type]
             return type_class(**args)
         else: