Olivier Dony (OpenERP) has proposed merging 
lp:~openerp-dev/openobject-server/trunk-float-rounding-odo into 
lp:openobject-server.

Requested reviews:
  OpenERP Core Team (openerp)

For more details, see:
https://code.launchpad.net/~openerp-dev/openobject-server/trunk-float-rounding-odo/+merge/82206

- Extracted res.currency's float computation methods to tools, so they can be 
reused cleanly for other floating-point operations.
- Also added a compare() method to allow for easy comparison of 2 float values, 
as suggested by Ferdinand on bug 867387
- Added tests and docstrings to explain the logic.

Watch out for compare(), because compare(amount1-amount2) == 0 is NOT the same 
as is_zero(amount1-amount2), this is explained in the docstrings and test. I 
think this is correct and the desired behavior.

The utility methods used in the YAML tests take redundant arguments due to 
idiotic namespace issues, feel free to suggest alternatives.
-- 
https://code.launchpad.net/~openerp-dev/openobject-server/trunk-float-rounding-odo/+merge/82206
Your team OpenERP R&D Team is subscribed to branch 
lp:~openerp-dev/openobject-server/trunk-float-rounding-odo.
=== modified file 'openerp/addons/base/res/res_currency.py'
--- openerp/addons/base/res/res_currency.py	2011-10-11 16:34:35 +0000
+++ openerp/addons/base/res/res_currency.py	2011-11-14 18:28:35 +0000
@@ -24,7 +24,7 @@
 from osv import fields, osv
 import tools
 
-from tools.misc import currency
+from tools.misc import currency, float_round, float_is_zero, float_compare
 from tools.translate import _
 
 CURRENCY_DISPLAY_PATTERN = re.compile(r'(\w+)\s*(?:\((.*)\))?')
@@ -127,15 +127,49 @@
         return [(x['id'], tools.ustr(x['name']) + (x['symbol'] and (' (' + tools.ustr(x['symbol']) + ')') or '')) for x in reads]
 
     def round(self, cr, uid, currency, amount):
-        if currency.rounding == 0:
-            return 0.0
-        else:
-            # /!\ First member below must be rounded to full unit!
-            # Do not pass a rounding digits value to round()
-            return round(amount / currency.rounding) * currency.rounding
+        """Return ``amount`` rounded  according to ``currency``'s
+           rounding rules.
+
+           :param browse_record currency: currency for which we are rounding
+           :param float amount: the amount to round
+           :return: rounded float
+        """
+        return float_round(amount, precision_rounding=currency.rounding)
+
+    def compare_amounts(self, cr, uid, currency, amount1, amount2):
+        """Compare ``amount1`` and ``amount2`` after rounding them according to the
+           given currency's precision..
+           An amount is considered lower/greater than another amount if their rounded
+           value is different. This is not the same as having a non-zero difference!
+
+           For example 1.432 and 1.431 are equal at 2 digits precision,
+           so this method would return 0.
+           However 0.006 and 0.002 are considered different (returns 1) because
+           they respectively round to 0.01 and 0.0, even though
+           0.006-0.002 = 0.004 which would be considered zero at 2 digits precision.
+
+           :param browse_record currency: currency for which we are rounding
+           :param float amount1: first amount to compare
+           :param float amount2: second amount to compare
+           :return: (resp.) -1, 0 or 1, if ``amount1`` is (resp.) lower than,
+                    equal to, or greater than ``amount2``, according to
+                    ``currency``'s rounding.
+        """
+        return float_compare(amount1, amount2, precision_rounding=currency.rounding)
 
     def is_zero(self, cr, uid, currency, amount):
-        return abs(self.round(cr, uid, currency, amount)) < currency.rounding
+        """Returns true if ``amount`` is small enough to be treated as
+           zero according to ``currency``'s rounding rules.
+
+           Warning: ``is_zero(amount1-amount2)`` is not always equivalent to 
+           ``compare_amounts(amount1,amount2) == 0``, as the former will round after
+           computing the difference, while the latter will round before, giving
+           different results for e.g. 0.006 and 0.002 at 2 digits precision.
+
+           :param browse_record currency: currency for which we are rounding
+           :param float amount: amount to compare with currency's zero
+        """
+        return float_is_zero(amount, precision_rounding=currency.rounding)
 
     def _get_conversion_rate(self, cr, uid, from_currency, to_currency, context=None):
         if context is None:

=== modified file 'openerp/addons/base/test/base_test.yml'
--- openerp/addons/base/test/base_test.yml	2011-07-28 08:27:52 +0000
+++ openerp/addons/base/test/base_test.yml	2011-11-14 18:28:35 +0000
@@ -144,3 +144,71 @@
     !python {model: res.partner.category}: |
         self.pool._init = True
 
+-
+    "Float precision tests: verify that float rounding methods are working correctly"
+-
+    !python {model: res.currency}: |
+        currency = self.browse(cr, uid, ref('base.EUR'))
+        def try_round(self, cr, currency, amount, expected):
+            result = str(self.round(cr, 1, currency, amount))
+            assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected)
+        try_round(self, cr, currency, 2.674,'2.67')
+        try_round(self, cr, currency, 2.675,'2.68') # in Python 2.7.2, round(2.675,2) gives 2.67
+        try_round(self, cr, currency, 0.001,'0.0')
+        try_round(self, cr, currency, 0.0049,'0.0') # 0.0049 is closer to 0 than to 0.01, so should round down
+        try_round(self, cr, currency, 0.005,'0.01') # the rule is to round half up
+
+        def try_zero(self, cr, currency, amount, expected):
+            assert self.is_zero(cr, 1, currency, amount) == expected, "Rounding error: %s should be zero!" % amount
+        try_zero(self, cr, currency, 0.01, False)
+        try_zero(self, cr, currency, 0.001, True)
+        try_zero(self, cr, currency, 0.0046, True)
+        try_zero(self, cr, currency, 2.68-2.675, False) # 2.68 - 2.675 = 0.005 -> rounds to 0.01
+        try_zero(self, cr, currency, 2.68-2.676, True) # 2.68 - 2.675 = 0.004 -> rounds to 0.0
+
+        def try_compare(self, cr, currency, amount1, amount2, expected):
+            assert self.compare_amounts(cr, 1, currency, amount1, amount2) == expected, \
+                "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected)
+        try_compare(self, cr, currency, 0.001, 0.001, 0)
+        try_compare(self, cr, currency, 0.001, 0.002, 0)
+        try_compare(self, cr, currency, 2.675, 2.68, 0)
+        try_compare(self, cr, currency, 2.676, 2.68, 0)
+        try_compare(self, cr, currency, 2.674, 2.68, -1)
+        try_compare(self, cr, currency, 3, 2.68, 1)
+        try_compare(self, cr, currency, 0.01, 0, 1)
+
+        from tools import float_compare, float_is_zero, float_round
+        def try_round_digits(float_round, amount, expected):
+            result = str(float_round(amount, precision_digits=3))
+            assert result == expected, 'Rounding error: got %s, expected %s' % (result, expected)
+        try_round_digits(float_round, 2.6745, '2.675')
+        try_round_digits(float_round, 2.6744, '2.674')
+        try_round_digits(float_round, 0.0004, '0.0')
+
+        def try_zero_digits(float_is_zero, amount, expected):
+            assert float_is_zero(amount, precision_digits=3) == expected, "Rounding error: %s should be zero!" % amount
+        try_zero_digits(float_is_zero, 0.0002, True)
+        try_zero_digits(float_is_zero, 0.00034, True)
+        try_zero_digits(float_is_zero, 0.0005, False)
+        try_zero_digits(float_is_zero, 0.0008, False)
+
+        def try_compare_digits(float_compare, amount1, amount2, expected):
+            assert float_compare(amount1, amount2, precision_digits=3) == expected, \
+                "Rounding error, compare_amounts(%s,%s) should be %s" % (amount1, amount2, expected)
+        try_compare_digits(float_compare, 0.0003, 0.0004, 0)
+        try_compare_digits(float_compare, 0.0002, 0.0005, -1)
+        try_compare_digits(float_compare, 0.0009, 0.0004, 1)
+
+        # specifying 2 precisions is illegal:
+        try:
+            float_is_zero(0.01, precision_digits=3, precision_rounding=0.01)
+        except AssertionError:
+            pass
+        try:
+            float_compare(0.01, 0.02, precision_digits=3, precision_rounding=0.01)
+        except AssertionError:
+            pass
+        try:
+            float_round(0.01, precision_digits=3, precision_rounding=0.01)
+        except AssertionError:
+            pass

=== modified file 'openerp/tools/misc.py'
--- openerp/tools/misc.py	2011-09-22 09:54:43 +0000
+++ openerp/tools/misc.py	2011-11-14 18:28:35 +0000
@@ -1200,4 +1200,96 @@
     def __missing__(self, key):
         return unquote(key)
 
-# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
+def _float_check_precision(precision_digits=None, precision_rounding=None):
+    assert (precision_digits is not None or precision_rounding is not None) and \
+        not (precision_digits and precision_rounding),\
+         "exactly one of precision_digits and precision_rounding must be specified"
+    if precision_digits is not None:
+        return 10 ** -precision_digits
+    return precision_rounding
+
+def float_round(amount, precision_digits=None, precision_rounding=None):
+    """Return ``amount`` rounded to ``precision_digits``
+       decimal digits, minimizing IEEE-854 floating point representation
+       errors.
+       Precision must be given by ``precision_digits`` or ``precision_rounding``,
+       not both!
+
+       To illustrate how this is different from the default round() builtin,
+       here is an example (depends on Python version, here is for v2.7.2 x64)::
+
+          >>> round_float(2.675)
+          2.68
+          >>> round(2.675,2)
+          2.67
+
+       :param float amount: the amount to round
+       :param int precision_digits: number of decimal digits to round to.
+       :param float precision_rounding: decimal number representing the minimum
+           non-zero value at the desired precision (for example, 0.01 for a 
+           2-digit precision).
+       :return: rounded float
+    """
+    rounding_factor = _float_check_precision(precision_digits=precision_digits,
+                                             precision_rounding=precision_rounding)
+    if rounding_factor == 0: return 0.0
+    # /!\ First member below must be rounded to full unit!
+    # Do not pass rounding digits to round()!
+    return round(amount / rounding_factor) * rounding_factor
+
+def float_is_zero(amount, precision_digits=None, precision_rounding=None):
+    """Returns true if ``amount`` is small enough to be treated as
+       zero at the given precision.
+       Precision must be given by ``precision_digits`` or ``precision_rounding``,
+       not both!
+
+       Warning: ``float_is_zero(amount1-amount2)`` is not always equivalent to 
+       ``float_compare(amount1,amount2) == 0``, as the former will round after
+       computing the difference, while the latter will round before, giving
+       different results for e.g. 0.006 and 0.002 at 2 digits precision. 
+
+       :param int precision_digits: number of decimal digits to round to.
+       :param float precision_rounding: decimal number representing the minimum
+           non-zero value at the desired precision (for example, 0.01 for a 
+           2-digit precision).
+       :param float amount: amount to compare with currency's zero
+       :return: True if ``amount`` is considered 0
+    """
+    rounding_factor = _float_check_precision(precision_digits=precision_digits,
+                                             precision_rounding=precision_rounding)
+    return abs(float_round(amount, precision_rounding=rounding_factor)) < rounding_factor
+
+def float_compare(amount1, amount2, precision_digits=None, precision_rounding=None):
+    """Compare ``amount1`` and ``amount2`` after rounding them according to the
+       given precision. An amount is considered lower/greater than another amount
+       if their rounded value is different. This is not the same as having a
+       non-zero difference!
+
+       For example 1.432 and 1.431 are equal at 2 digits precision,
+       so this method would return 0
+       However 0.006 and 0.002 are considered different (returns 1) because
+       they respectively round to 0.01 and 0.0, even though
+       0.006-0.002 = 0.004 which would be considered zero at 2 digits precision.
+
+
+       Precision must be given by ``precision_digits`` or ``precision_rounding``,
+       not both!
+
+       :param int precision_digits: number of decimal digits to round to.
+       :param float precision_rounding: decimal number representing the minimum
+           non-zero value at the desired precision (for example, 0.01 for a 
+           2-digit precision).
+       :param float amount1: first amount to compare
+       :param float amount2: second amount to compare
+       :return: (resp.) -1, 0 or 1, if ``amount1`` is (resp.) lower than,
+           equal to, or greater than ``amount2``, at the given precision.
+    """
+    rounding_factor = _float_check_precision(precision_digits=precision_digits,
+                                             precision_rounding=precision_rounding)
+    amount1 = float_round(amount1, precision_rounding=rounding_factor)
+    amount2 = float_round(amount2, precision_rounding=rounding_factor)
+    delta = amount1 - amount2
+    if float_is_zero(delta, precision_rounding=rounding_factor): return 0
+    return -1 if delta < 0.0 else 1
+
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
\ No newline at end of file

_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help   : https://help.launchpad.net/ListHelp

Reply via email to