launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-round into launchpad:master.

Commit message:
Work around Python 3's round() semantics

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/395908

Python 3 changed the built-in round() function to use round-to-even for exact half-way cases.  While this eliminates a bias away from zero and is thus a better choice in many cases, it's not what several parts of Launchpad expected.  Use the decimal module instead, which produces identical results for our purposes on both Python 2 and 3.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-round into launchpad:master.

diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
index 89dee84..268b5bd 100644
--- a/lib/lp/app/browser/tales.py
+++ b/lib/lp/app/browser/tales.py
@@ -72,6 +72,7 @@ from lp.registry.interfaces.distributionsourcepackage import (
 from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.services.utils import round_half_up
 from lp.services.webapp.authorization import check_permission
 from lp.services.webapp.canonicalurl import nearest_adapter
 from lp.services.webapp.error import SystemErrorView
@@ -2467,7 +2468,7 @@ class DurationFormatterAPI:
         # Convert seconds into minutes, and round it.
         minutes, remaining_seconds = divmod(seconds, 60)
         minutes += remaining_seconds / 60.0
-        minutes = int(round(minutes))
+        minutes = round_half_up(minutes)
 
         if minutes <= 59:
             return "%d minutes" % minutes
@@ -2480,7 +2481,7 @@ class DurationFormatterAPI:
         # greater than one hour, but fewer than ten hours, to a 10
         # minute granularity.
         hours, remaining_seconds = divmod(seconds, 3600)
-        ten_minute_chunks = int(round(remaining_seconds / 600.0))
+        ten_minute_chunks = round_half_up(remaining_seconds / 600.0)
         minutes = ten_minute_chunks * 10
         hours += (minutes / 60)
         minutes %= 60
@@ -2501,7 +2502,7 @@ class DurationFormatterAPI:
 
         # Try to calculate the approximate number of hours, to a
         # maximum of 47.
-        hours = int(round(seconds / 3600.0))
+        hours = round_half_up(seconds / 3600.0)
         if hours <= 47:
             return "%d hours" % hours
 
@@ -2511,7 +2512,7 @@ class DurationFormatterAPI:
 
         # Try to approximate to day granularity, up to a maximum of 13
         # days.
-        days = int(round(seconds / (24 * 3600)))
+        days = round_half_up(seconds / (24 * 3600))
         if days <= 13:
             return "%s days" % days
 
@@ -2521,7 +2522,7 @@ class DurationFormatterAPI:
 
         # If we've made it this far, we'll calculate the duration to a
         # granularity of weeks, once and for all.
-        weeks = int(round(seconds / (7 * 24 * 3600.0)))
+        weeks = round_half_up(seconds / (7 * 24 * 3600.0))
         return "%d weeks" % weeks
 
     def millisecondduration(self):
diff --git a/lib/lp/app/widgets/date.py b/lib/lp/app/widgets/date.py
index 2d7d119..bbeca15 100644
--- a/lib/lp/app/widgets/date.py
+++ b/lib/lp/app/widgets/date.py
@@ -37,6 +37,7 @@ from zope.formlib.textwidgets import TextWidget
 from zope.formlib.widget import DisplayWidget
 
 from lp.app.validators import LaunchpadValidationError
+from lp.services.utils import round_half_up
 from lp.services.webapp.escaping import html_escape
 from lp.services.webapp.interfaces import ILaunchBag
 
@@ -390,9 +391,9 @@ class DateTimeWidget(TextWidget):
         try:
             year, month, day, hour, minute, second, dummy_tz = parse(input)
             second, micro = divmod(second, 1.0)
-            micro = round(micro * 1000000)
+            micro = round_half_up(micro * 1000000)
             dt = datetime(year, month, day,
-                          hour, minute, int(second), int(micro))
+                          hour, minute, int(second), micro)
         except (DateTimeError, ValueError, IndexError) as v:
             raise ConversionError('Invalid date value', v)
         return self.time_zone.localize(dt)
diff --git a/lib/lp/app/widgets/textwidgets.py b/lib/lp/app/widgets/textwidgets.py
index ad4cf43..23b3f5f 100644
--- a/lib/lp/app/widgets/textwidgets.py
+++ b/lib/lp/app/widgets/textwidgets.py
@@ -18,6 +18,7 @@ from zope.formlib.textwidgets import (
     )
 
 from lp.app.errors import UnexpectedFormData
+from lp.services.utils import round_half_up
 
 
 class StrippedTextWidget(TextWidget):
@@ -104,9 +105,9 @@ class LocalDateTimeWidget(TextWidget):
         try:
             year, month, day, hour, minute, second, dummy_tz = parse(input)
             second, micro = divmod(second, 1.0)
-            micro = round(micro * 1000000)
+            micro = round_half_up(micro * 1000000)
             dt = datetime.datetime(year, month, day,
-                                   hour, minute, int(second), int(micro))
+                                   hour, minute, int(second), micro)
         except (DateTimeError, ValueError, IndexError) as v:
             raise ConversionError('Invalid date value', v)
         tz = pytz.timezone(self.timeZoneName)
diff --git a/lib/lp/services/tests/test_utils.py b/lib/lp/services/tests/test_utils.py
index 959f718..d6cb1ec 100644
--- a/lib/lp/services/tests/test_utils.py
+++ b/lib/lp/services/tests/test_utils.py
@@ -32,6 +32,7 @@ from lp.services.utils import (
     iter_split,
     load_bz2_pickle,
     obfuscate_structure,
+    round_half_up,
     run_capturing_output,
     sanitise_urls,
     save_bz2_pickle,
@@ -422,3 +423,29 @@ class TestSanitiseURLs(TestCase):
                 '{"one": "http://example.com/";, '
                 '"two": "http://alice:secret@xxxxxxxxxxx/";, '
                 '"three": "http://bob:hidden@xxxxxxxxxxx/"}'))
+
+
+class TestRoundHalfUp(TestCase):
+
+    def test_exact_integer(self):
+        self.assertEqual(-2, round_half_up(-2.0))
+        self.assertEqual(-1, round_half_up(-1.0))
+        self.assertEqual(0, round_half_up(0.0))
+        self.assertEqual(1, round_half_up(1.0))
+        self.assertEqual(2, round_half_up(2.0))
+
+    def test_not_half(self):
+        self.assertEqual(-999, round_half_up(-999.1))
+        self.assertEqual(-999, round_half_up(-998.9))
+        self.assertEqual(0, round_half_up(-0.4))
+        self.assertEqual(0, round_half_up(0.3))
+        self.assertEqual(75, round_half_up(74.7))
+        self.assertEqual(75, round_half_up(75.2))
+
+    def test_half(self):
+        self.assertEqual(-10, round_half_up(-9.5))
+        self.assertEqual(-9, round_half_up(-8.5))
+        self.assertEqual(-1, round_half_up(-0.5))
+        self.assertEqual(1, round_half_up(0.5))
+        self.assertEqual(9, round_half_up(8.5))
+        self.assertEqual(10, round_half_up(9.5))
diff --git a/lib/lp/services/utils.py b/lib/lp/services/utils.py
index a9dcd75..6146e3e 100644
--- a/lib/lp/services/utils.py
+++ b/lib/lp/services/utils.py
@@ -20,6 +20,7 @@ __all__ = [
     'obfuscate_email',
     'obfuscate_structure',
     're_email_address',
+    'round_half_up',
     'run_capturing_output',
     'sanitise_urls',
     'save_bz2_pickle',
@@ -32,6 +33,7 @@ __all__ = [
 
 import bz2
 from datetime import datetime
+import decimal
 from itertools import (
     islice,
     tee,
@@ -364,3 +366,15 @@ def sanitise_urls(s):
     # Remove credentials from URLs.
     password_re = re.compile(r'://([^:@/]*:[^@/]*@)(\S+)')
     return password_re.sub(r'://<redacted>@\2', s)
+
+
+def round_half_up(number):
+    """Round `number` to the nearest integer, with ties going away from zero.
+
+    This is equivalent to `int(round(number))` on Python 2; Python 3's
+    `round` prefers round-to-even in the case of ties, which does a better
+    job of avoiding statistical bias in many cases but isn't always what we
+    want.
+    """
+    return int(decimal.Decimal(number).to_integral_value(
+        rounding=decimal.ROUND_HALF_UP))
diff --git a/lib/lp/translations/model/translationsoverview.py b/lib/lp/translations/model/translationsoverview.py
index 4e789b0..04bdec9 100644
--- a/lib/lp/translations/model/translationsoverview.py
+++ b/lib/lp/translations/model/translationsoverview.py
@@ -16,6 +16,7 @@ from lp.services.database.sqlbase import (
     cursor,
     sqlvalues,
     )
+from lp.services.utils import round_half_up
 from lp.translations.interfaces.translationsoverview import (
     ITranslationsOverview,
     MalformedKarmaCacheData,
@@ -43,9 +44,9 @@ class TranslationsOverview:
 
         normalized_sizes = []
         for (pillar, size) in pillars:
-            new_size = int(round(
+            new_size = round_half_up(
                 real_minimum +
-                (size - offset - real_minimum) * multiplier))
+                (size - offset - real_minimum) * multiplier)
             normalized_sizes.append({'pillar': pillar, 'weight': new_size})
         return normalized_sizes