launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/approximateduration-no-words into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #138490 in Launchpad itself: "approximateduration shouldn't spell out numbers <10"
  https://bugs.launchpad.net/launchpad/+bug/138490

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/approximateduration-no-words/+merge/66586

Utterly rip out all uses (as well as the code itself) of approximateduration(use_words) and fmt:approximateduration/use-digits.
-- 
https://code.launchpad.net/~stevenk/launchpad/approximateduration-no-words/+merge/66586
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/approximateduration-no-words into lp:launchpad.

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2011-06-30 13:11:41 +0000
+++ lib/lp/app/browser/tales.py	2011-07-01 11:44:31 +0000
@@ -2144,12 +2144,7 @@
         if name == 'exactduration':
             return self.exactduration()
         elif name == 'approximateduration':
-            use_words = True
-            if len(furtherPath) == 1:
-                if 'use-digits' == furtherPath[0]:
-                    furtherPath.pop()
-                    use_words = False
-            return self.approximateduration(use_words)
+            return self.approximateduration()
         else:
             raise TraversalError(name)
 
@@ -2179,19 +2174,13 @@
 
         return ', '.join(parts)
 
-    def approximateduration(self, use_words=True):
+    def approximateduration(self):
         """Return a nicely-formatted approximate duration.
 
-        E.g. 'an hour', 'three minutes', '1 hour 10 minutes' and so
+        E.g. 'an hour', '3 minutes', '1 hour 10 minutes' and so
         forth.
 
         See https://launchpad.canonical.com/PresentingLengthsOfTime.
-
-        :param use_words: Specificly determines whether or not to use
-            words for numbers less than or equal to ten.  Expanding
-            numbers to words makes sense when the number is used in
-            prose or a singualar item on a page, but when used in
-            a table, the words do not work as well.
         """
         # NOTE: There are quite a few "magic numbers" in this
         # implementation; they are generally just figures pulled
@@ -2224,54 +2213,29 @@
             (35, '30 seconds'),
             (45, '40 seconds'),
             (55, '50 seconds'),
-            (90, 'a minute'),
+            (90, '1 minute'),
         ]
-        if not use_words:
-            representation_in_seconds[-1] = (90, '1 minute')
 
         # Break representation_in_seconds into two pieces, to simplify
         # finding the correct display value, through the use of the
         # built-in bisect module.
         second_boundaries, display_values = zip(*representation_in_seconds)
 
-        # Is seconds small enough that we can produce a representation
-        # in seconds (up to 'a minute'?)
-        if seconds < second_boundaries[-1]:
-            # Use the built-in bisection algorithm to locate the index
-            # of the item which "seconds" sorts after.
-            matching_element_index = bisect.bisect(second_boundaries, seconds)
-
-            # Return the corresponding display value.
-            return display_values[matching_element_index]
-
-        # More than a minute, approximately; our calculation strategy
-        # changes. From this point forward, we may also need a
-        # "verbal" representation of the number. (We never need a
-        # verbal representation of "1", because we tend to special
-        # case the number 1 for various approximations, and we usually
-        # use a word like "an", instead of "one", e.g. "an hour")
-        if use_words:
-            number_name = {
-                2: 'two', 3: 'three', 4: 'four', 5: 'five',
-                6: 'six', 7: 'seven', 8: 'eight', 9: 'nine',
-                10: 'ten'}
-        else:
-            number_name = dict((number, number) for number in range(2, 11))
+        number_name = dict((number, number) for number in range(2, 11))
 
         # Convert seconds into minutes, and round it.
         minutes, remaining_seconds = divmod(seconds, 60)
         minutes += remaining_seconds / 60.0
         minutes = int(round(minutes))
 
-        if minutes <= 59:
+        if minutes == 1:
+            return "1 minute"
+        elif minutes <= 59:
             return "%s minutes" % number_name.get(minutes, str(minutes))
 
         # Is the duration less than an hour and 5 minutes?
         if seconds < (60 + 5) * 60:
-            if use_words:
-                return "an hour"
-            else:
-                return "1 hour"
+            return "1 hour"
 
         # Next phase: try and calculate an approximate duration
         # greater than one hour, but fewer than ten hours, to a 10

=== modified file 'lib/lp/app/doc/tales.txt'
--- lib/lp/app/doc/tales.txt	2011-06-23 13:10:40 +0000
+++ lib/lp/app/doc/tales.txt	2011-07-01 11:44:31 +0000
@@ -1409,9 +1409,6 @@
     '2 days, 0 hours, 0 minutes, 0.0 seconds'
 
     >>> test_tales('delta/fmt:approximateduration', delta=delta)
-    'two days'
-
-    >>> test_tales('delta/fmt:approximateduration/use-digits', delta=delta)
     '2 days'
 
     >>> delta = timedelta(days=12, hours=6, minutes=30)
@@ -1421,17 +1418,11 @@
     >>> test_tales('delta/fmt:approximateduration', delta=delta)
     '12 days'
 
-    >>> test_tales('delta/fmt:approximateduration/use-digits', delta=delta)
-    '12 days'
-
     >>> delta = timedelta(days=0, minutes=62)
     >>> test_tales('delta/fmt:exactduration', delta=delta)
     '1 hour, 2 minutes, 0.0 seconds'
 
     >>> test_tales('delta/fmt:approximateduration', delta=delta)
-    'an hour'
-
-    >>> test_tales('delta/fmt:approximateduration/use-digits', delta=delta)
     '1 hour'
 
     >>> delta = timedelta(days=0, minutes=82)
@@ -1441,17 +1432,11 @@
     >>> test_tales('delta/fmt:approximateduration', delta=delta)
     '1 hour 20 minutes'
 
-    >>> test_tales('delta/fmt:approximateduration/use-digits', delta=delta)
-    '1 hour 20 minutes'
-
     >>> delta = timedelta(days=0, seconds=62)
     >>> test_tales('delta/fmt:exactduration', delta=delta)
     '1 minute, 2.0 seconds'
 
     >>> test_tales('delta/fmt:approximateduration', delta=delta)
-    'a minute'
-
-    >>> test_tales('delta/fmt:approximateduration/use-digits', delta=delta)
     '1 minute'
 
     >>> delta = timedelta(days=0, seconds=90)
@@ -1459,9 +1444,6 @@
     '1 minute, 30.0 seconds'
 
     >>> test_tales('delta/fmt:approximateduration', delta=delta)
-    'two minutes'
-
-    >>> test_tales('delta/fmt:approximateduration/use-digits', delta=delta)
     '2 minutes'
 
 

=== modified file 'lib/lp/bugs/templates/bugtarget-patches.pt'
--- lib/lp/bugs/templates/bugtarget-patches.pt	2011-02-03 20:54:43 +0000
+++ lib/lp/bugs/templates/bugtarget-patches.pt	2011-07-01 11:44:31 +0000
@@ -75,7 +75,7 @@
                              age python:view.patchAge(patch)"
                  tal:attributes="id string:patch-cell-${repeat/patch_task/index}">
                    <a tal:attributes="href python:view.proxiedUrlForLibraryFile(patch)"
-                      tal:content="age/fmt:approximateduration/use-digits"></a>
+                      tal:content="age/fmt:approximateduration"></a>
                <div class="popupTitle"
                     tal:attributes="id string:patch-popup-${repeat/patch_task/index};">
                  <p tal:define="submitter patch/message/owner">