[PATCH] build: Fix potential type error when generating human-friendly byte count strings.

[PATCH] build: Fix potential type error when generating human-friendly byte count strings.

[Steve Sprang]

[-- Attachment #1.1: Type: text/plain, Size: 199 bytes --]

This is a follow up tweak to my previous "progress bar" patch. With a
really slow throughput it's possible to get fractional sub-KiB byte counts,
so I added some additional number massaging.

-Steve

[-- Attachment #1.2: Type: text/html, Size: 255 bytes --]

[-- Attachment #2: type-fix.patch --]
[-- Type: text/x-patch, Size: 1972 bytes --]

From daccd58e5fbe366ebcd33092f70ca45bc79285e0 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Wed, 9 Sep 2015 13:59:52 -0700
Subject: [PATCH] build: Fix potential type error when generating
 human-friendly byte count strings.

* guix/build/download.scm (number->integer): New function.
  (seconds->string): Use new rounding function.
  (byte-count->string):  Use new rounding function.
---
 guix/build/download.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/guix/build/download.scm b/guix/build/download.scm
index 6e85174..a0b8d92 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -49,6 +49,10 @@
   ;; Size of the HTTP receive buffer.
   65536)
 
+(define (number->integer n)
+  "Given an arbitrary number N, round it and return the exact result."
+  (inexact->exact (round n)))
+
 (define (duration->seconds duration)
   "Return the number of seconds represented by DURATION, a 'time-duration'
 object, as an inexact number."
@@ -60,7 +64,7 @@ object, as an inexact number."
 format."
   (if (not (number? duration))
       "00:00:00"
-      (let* ((total-seconds (inexact->exact (round duration)))
+      (let* ((total-seconds (number->integer duration))
              (extra-seconds (modulo total-seconds 3600))
              (hours         (quotient total-seconds 3600))
              (mins          (quotient extra-seconds 60))
@@ -75,8 +79,8 @@ way."
         (GiB (expt 1024. 3))
         (TiB (expt 1024. 4)))
     (cond
-     ((< size KiB) (format #f "~dB" (inexact->exact size)))
-     ((< size MiB) (format #f "~dKiB" (inexact->exact (round (/ size KiB)))))
+     ((< size KiB) (format #f "~dB"     (number->integer size)))
+     ((< size MiB) (format #f "~dKiB"   (number->integer (/ size KiB))))
      ((< size GiB) (format #f "~,1fMiB" (/ size MiB)))
      ((< size TiB) (format #f "~,2fGiB" (/ size GiB)))
      (else         (format #f "~,3fTiB" (/ size TiB))))))
-- 
2.5.0

Re: [PATCH] build: Fix potential type error when generating human-friendly byte count strings.

[Mark H Weaver]

Steve Sprang <steve.sprang@gmail.com> writes:

> This is a follow up tweak to my previous "progress bar" patch. With a
> really slow throughput it's possible to get fractional sub-KiB byte
> counts, so I added some additional number massaging.

Sounds good!  Looks good to me except for a few minor nits on this
auxiliary procedure:

> +(define (number->integer n)
> +  "Given an arbitrary number N, round it and return the exact result."
> +  (inexact->exact (round n)))

How about calling it "nearest-exact-integer"?  Also, it makes sense only
for real numbers, not arbitrary numbers, and the variable name N is
conventionally used to denote natural numbers, and X for real numbers.

So, how about naming the argument 'x', and using something closer to the
following docstring:

  "Given a real number X, return the nearest exact integer, with ties
going to the nearest exact even integer."

Otherwise, looks good to me.  Can you send an updated patch?

      Mark

Re: [PATCH] build: Fix potential type error when generating human-friendly byte count strings.

[Steve Sprang]

[-- Attachment #1.1: Type: text/plain, Size: 1118 bytes --]

Sure thing. Here you go.

-Steve

On Wed, Sep 9, 2015 at 7:42 PM, Mark H Weaver <mhw@netris.org> wrote:

> Steve Sprang <steve.sprang@gmail.com> writes:
>
> > This is a follow up tweak to my previous "progress bar" patch. With a
> > really slow throughput it's possible to get fractional sub-KiB byte
> > counts, so I added some additional number massaging.
>
> Sounds good!  Looks good to me except for a few minor nits on this
> auxiliary procedure:
>
> > +(define (number->integer n)
> > +  "Given an arbitrary number N, round it and return the exact result."
> > +  (inexact->exact (round n)))
>
> How about calling it "nearest-exact-integer"?  Also, it makes sense only
> for real numbers, not arbitrary numbers, and the variable name N is
> conventionally used to denote natural numbers, and X for real numbers.
>
> So, how about naming the argument 'x', and using something closer to the
> following docstring:
>
>   "Given a real number X, return the nearest exact integer, with ties
> going to the nearest exact even integer."
>
> Otherwise, looks good to me.  Can you send an updated patch?
>
>       Mark
>

[-- Attachment #1.2: Type: text/html, Size: 1726 bytes --]

[-- Attachment #2: type-fix2.patch --]
[-- Type: text/x-patch, Size: 2045 bytes --]

From 922d9ae8b6f2c13638b49fcc8b79f8d464f01244 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Wed, 9 Sep 2015 13:59:52 -0700
Subject: [PATCH] build: Fix potential type error when generating
 human-friendly byte count strings.

* guix/build/download.scm (nearest-exact-integer): New function.
  (seconds->string): Use new rounding function.
  (byte-count->string):  Use new rounding function.
---
 guix/build/download.scm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/guix/build/download.scm b/guix/build/download.scm
index 6e85174..31d60fb 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -49,6 +49,11 @@
   ;; Size of the HTTP receive buffer.
   65536)
 
+(define (nearest-exact-integer x)
+  "Given a real number X, return the nearest exact integer, with ties going to
+the nearest exact even integer."
+  (inexact->exact (round x)))
+
 (define (duration->seconds duration)
   "Return the number of seconds represented by DURATION, a 'time-duration'
 object, as an inexact number."
@@ -60,7 +65,7 @@ object, as an inexact number."
 format."
   (if (not (number? duration))
       "00:00:00"
-      (let* ((total-seconds (inexact->exact (round duration)))
+      (let* ((total-seconds (nearest-exact-integer duration))
              (extra-seconds (modulo total-seconds 3600))
              (hours         (quotient total-seconds 3600))
              (mins          (quotient extra-seconds 60))
@@ -75,8 +80,8 @@ way."
         (GiB (expt 1024. 3))
         (TiB (expt 1024. 4)))
     (cond
-     ((< size KiB) (format #f "~dB" (inexact->exact size)))
-     ((< size MiB) (format #f "~dKiB" (inexact->exact (round (/ size KiB)))))
+     ((< size KiB) (format #f "~dB"     (nearest-exact-integer size)))
+     ((< size MiB) (format #f "~dKiB"   (nearest-exact-integer (/ size KiB))))
      ((< size GiB) (format #f "~,1fMiB" (/ size MiB)))
      ((< size TiB) (format #f "~,2fGiB" (/ size GiB)))
      (else         (format #f "~,3fTiB" (/ size TiB))))))
-- 
2.5.0

Re: [PATCH] build: Fix potential type error when generating human-friendly byte count strings.

[Steve Sprang]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

Hi Mark,

Does the updated patch look okay? I'm making some related changes and it
would be easier if this patch was integrated.

Thanks!
-Steve


On Wed, Sep 9, 2015 at 8:31 PM, Steve Sprang <steve.sprang@gmail.com> wrote:

> Sure thing. Here you go.
>
> -Steve
>
> On Wed, Sep 9, 2015 at 7:42 PM, Mark H Weaver <mhw@netris.org> wrote:
>
>> Steve Sprang <steve.sprang@gmail.com> writes:
>>
>> > This is a follow up tweak to my previous "progress bar" patch. With a
>> > really slow throughput it's possible to get fractional sub-KiB byte
>> > counts, so I added some additional number massaging.
>>
>> Sounds good!  Looks good to me except for a few minor nits on this
>> auxiliary procedure:
>>
>> > +(define (number->integer n)
>> > +  "Given an arbitrary number N, round it and return the exact result."
>> > +  (inexact->exact (round n)))
>>
>> How about calling it "nearest-exact-integer"?  Also, it makes sense only
>> for real numbers, not arbitrary numbers, and the variable name N is
>> conventionally used to denote natural numbers, and X for real numbers.
>>
>> So, how about naming the argument 'x', and using something closer to the
>> following docstring:
>>
>>   "Given a real number X, return the nearest exact integer, with ties
>> going to the nearest exact even integer."
>>
>> Otherwise, looks good to me.  Can you send an updated patch?
>>
>>       Mark
>>
>
>

[-- Attachment #2: Type: text/html, Size: 2461 bytes --]

Re: [PATCH] build: Fix potential type error when generating human-friendly byte count strings.

[Mark H Weaver]

Steve Sprang <steve.sprang@gmail.com> writes:
> Does the updated patch look okay? I'm making some related changes and
> it would be easier if this patch was integrated.

Pushed to 'master' with some modifications to the commit log.
Sorry for the delay.

    Thanks!
      Mark