emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Support time units in est+
@ 2017-01-18 20:33 Malcolm Matalka
  2017-01-18 21:13 ` Nick Dokos
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Malcolm Matalka @ 2017-01-18 20:33 UTC (permalink / raw)
  To: emacs-orgmode

Hey, this is my first elisp programming so I'm quite certain this is a
big hack.  I just stole elements from elsewhere in the file.  I'm hoping
this is good enough to get accepted then perhaps someone with more taste
would be able to refactor it to be a bit better.

Let me know if I need to change anything.

From 1167bd20e042ad2ae3f2712f596d76ad8b230336 Mon Sep 17 00:00:00 2001
From: orbitz <orbitz@gmail.com>
Date: Wed, 18 Jan 2017 21:18:23 +0100
Subject: [PATCH] org-colview.el: Add support for time units to est+

* lisp/org-colview.el: Add support for time units in est+.  Ranges are interpreted just like non-range estimates.

TINYCHANGE
---
lisp/org-colview.el | 44 ++++++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 45c71a028..2a5c067ac 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1288,23 +1288,43 @@ When PRINTF is non-nil, use it to format the result."
    (/ (apply #'+ (mapcar #'org-columns--age-to-seconds ages))
       (float (length ages)))))
 
-(defun org-columns--summary-estimate (estimates printf)
+(defun org-columns--summary-estimate (estimates _)
   "Combine a list of estimates, using mean and variance.
The mean and variance of the result will be the sum of the means
 and variances (respectively) of the individual estimates."
   (let ((mean 0)
-        (var 0))
+        (var 0)
+        (hms-flag nil)
+        (duration-flag nil))
     (dolist (e estimates)
-      (pcase (mapcar #'string-to-number (split-string e "-"))
-(`(,low ,high)
- (let ((m (/ (+ low high) 2.0)))
-   (cl-incf mean m)
-   (cl-incf var (- (/ (+ (* low low) (* high high)) 2.0) (* m m)))))
-(`(,value) (cl-incf mean value))))
-    (let ((sd (sqrt var)))
-      (format "%s-%s"
-      (format (or printf "%.0f") (- mean sd))
-      (format (or printf "%.0f") (+ mean sd))))))
+      (pcase (split-string e "-")
+        (`(,low ,high)
+         (dolist (time (list high low))
+           (cond
+            (duration-flag)
+            ((string-match-p org-columns--duration-re time)
+             (setq duration-flag t))
+            (hms-flag)
+            ((string-match-p "\\`[0-9]+:[0-9]+:[0-9]+\\'" time)
+             (setq hms-flag t))))
+         (let* ((low-sec (org-columns--time-to-seconds low))
+                (high-sec (org-columns--time-to-seconds high))
+                (m (/ (+ low-sec high-sec) 2.0)))
+           (cl-incf mean m)
+           (cl-incf var (- (/ (+ (* low-sec low-sec) (* high-sec high-sec)) 2.0) (* m m)))))
+        (`(,value) (cl-incf mean (org-columns--time-to-seconds value)))))
+    (let* ((sd (sqrt var))
+           (low-second (truncate (- mean sd)))
+           (high-second (truncate (+ mean sd)))
+           (low
+            (cond (duration-flag (org-minutes-to-clocksum-string (/ low-second 60.0)))
+                  (hms-flag (format-seconds "%h:%.2m:%.2s" low-second))
+                  (t (format-seconds "%h:%.2m" low-second))))
+           (high
+            (cond (duration-flag (org-minutes-to-clocksum-string (/  high-second 60.0)))
+                  (hms-flag (format-seconds "%h:%.2m:%.2s" high-second))
+                  (t (format-seconds "%h:%.2m" high-second)))))
+      (format "%s-%s" low high))))
 
 
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support time units in est+
  2017-01-18 20:33 [PATCH] Support time units in est+ Malcolm Matalka
@ 2017-01-18 21:13 ` Nick Dokos
       [not found]   ` <CAKziXDUDhHJWmfm2kN+tM2WmXPiqEXTfZtQYVriCBydRqiiW7g@mail.gmail.com>
  2017-02-03 12:56 ` Malcolm Matalka
  2017-02-04 14:30 ` Nicolas Goaziou
  2 siblings, 1 reply; 6+ messages in thread
From: Nick Dokos @ 2017-01-18 21:13 UTC (permalink / raw)
  To: emacs-orgmode

Malcolm Matalka <mmatalka@gmail.com> writes:

> Hey, this is my first elisp programming so I'm quite certain this is a
> big hack.  I just stole elements from elsewhere in the file.  I'm hoping
> this is good enough to get accepted then perhaps someone with more taste
> would be able to refactor it to be a bit better.
>
> Let me know if I need to change anything.
>

Yes, indeed: the first thing you should do is explain what you are
trying to do.  E.g. what does "est+" mean? Once you have explained
*what* you are trying to do, then somebody might be able to suggest
*how* to do it better (if it is a worthwhile thing to do in the first place).

> From 1167bd20e042ad2ae3f2712f596d76ad8b230336 Mon Sep 17 00:00:00 2001
> From: orbitz <orbitz@gmail.com>
> Date: Wed, 18 Jan 2017 21:18:23 +0100
> Subject: [PATCH] org-colview.el: Add support for time units to est+
>
> * lisp/org-colview.el: Add support for time units in est+.  Ranges are interpreted just like non-range estimates.
>
> TINYCHANGE
> ---
> lisp/org-colview.el | 44 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lisp/org-colview.el b/lisp/org-colview.el
> index 45c71a028..2a5c067ac 100644
> --- a/lisp/org-colview.el
> +++ b/lisp/org-colview.el
> @@ -1288,23 +1288,43 @@ When PRINTF is non-nil, use it to format the result."
>     (/ (apply #'+ (mapcar #'org-columns--age-to-seconds ages))
>        (float (length ages)))))
>  
> -(defun org-columns--summary-estimate (estimates printf)
> +(defun org-columns--summary-estimate (estimates _)
>    "Combine a list of estimates, using mean and variance.
> The mean and variance of the result will be the sum of the means
>  and variances (respectively) of the individual estimates."
>    (let ((mean 0)
> -        (var 0))
> +        (var 0)
> +        (hms-flag nil)
> +        (duration-flag nil))
>      (dolist (e estimates)
> -      (pcase (mapcar #'string-to-number (split-string e "-"))
> -(`(,low ,high)
> - (let ((m (/ (+ low high) 2.0)))
> -   (cl-incf mean m)
> -   (cl-incf var (- (/ (+ (* low low) (* high high)) 2.0) (* m m)))))
> -(`(,value) (cl-incf mean value))))
> -    (let ((sd (sqrt var)))
> -      (format "%s-%s"
> -      (format (or printf "%.0f") (- mean sd))
> -      (format (or printf "%.0f") (+ mean sd))))))
> +      (pcase (split-string e "-")
> +        (`(,low ,high)
> +         (dolist (time (list high low))
> +           (cond
> +            (duration-flag)
> +            ((string-match-p org-columns--duration-re time)
> +             (setq duration-flag t))
> +            (hms-flag)
> +            ((string-match-p "\\`[0-9]+:[0-9]+:[0-9]+\\'" time)
> +             (setq hms-flag t))))
> +         (let* ((low-sec (org-columns--time-to-seconds low))
> +                (high-sec (org-columns--time-to-seconds high))
> +                (m (/ (+ low-sec high-sec) 2.0)))
> +           (cl-incf mean m)
> +           (cl-incf var (- (/ (+ (* low-sec low-sec) (* high-sec high-sec)) 2.0) (* m m)))))
> +        (`(,value) (cl-incf mean (org-columns--time-to-seconds value)))))
> +    (let* ((sd (sqrt var))
> +           (low-second (truncate (- mean sd)))
> +           (high-second (truncate (+ mean sd)))
> +           (low
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/ low-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" low-second))
> +                  (t (format-seconds "%h:%.2m" low-second))))
> +           (high
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/  high-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" high-second))
> +                  (t (format-seconds "%h:%.2m" high-second)))))
> +      (format "%s-%s" low high))))

-- 
Nick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support time units in est+
       [not found]     ` <CAKziXDX_3=UTBLM8v=HZJSCR2p6tqiKwSgF+a-EXvRhsp+tAjw@mail.gmail.com>
@ 2017-01-19  4:49       ` Malcolm Matalka
  2017-01-19 15:44         ` Nick Dokos
  0 siblings, 1 reply; 6+ messages in thread
From: Malcolm Matalka @ 2017-01-19  4:49 UTC (permalink / raw)
  To: Nick Dokos; +Cc: emacs-orgmode

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

Den 18 jan. 2017 22:19 skrev "Nick Dokos" <ndokos@gmail.com>:

Malcolm Matalka <mmatalka@gmail.com> writes:

> Hey, this is my first elisp programming so I'm quite certain this is a
> big hack.  I just stole elements from elsewhere in the file.  I'm hoping
> this is good enough to get accepted then perhaps someone with more taste
> would be able to refactor it to be a bit better.
>
> Let me know if I need to change anything.
>

Yes, indeed: the first thing you should do is explain what you are
trying to do.  E.g. what does "est+" mean? Once you have explained
*what* you are trying to do, then somebody might be able to suggest
*how* to do it better (if it is a worthwhile thing to do in the first
place).


I'm not sure I understand your question.  est+ is an existing functionality
in columnview that is documented. I've brought it closer to the regular
effort functionality by supporting time units.


> From 1167bd20e042ad2ae3f2712f596d76ad8b230336 Mon Sep 17 00:00:00 2001
> From: orbitz <orbitz@gmail.com>
> Date: Wed, 18 Jan 2017 21:18:23 +0100
> Subject: [PATCH] org-colview.el: Add support for time units to est+
>
> * lisp/org-colview.el: Add support for time units in est+.  Ranges are
interpreted just like non-range estimates.
>
> TINYCHANGE
> ---
> lisp/org-colview.el | 44 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lisp/org-colview.el b/lisp/org-colview.el
> index 45c71a028..2a5c067ac 100644
> --- a/lisp/org-colview.el
> +++ b/lisp/org-colview.el
> @@ -1288,23 +1288,43 @@ When PRINTF is non-nil, use it to format the
result."
>     (/ (apply #'+ (mapcar #'org-columns--age-to-seconds ages))
>        (float (length ages)))))
>
> -(defun org-columns--summary-estimate (estimates printf)
> +(defun org-columns--summary-estimate (estimates _)
>    "Combine a list of estimates, using mean and variance.
> The mean and variance of the result will be the sum of the means
>  and variances (respectively) of the individual estimates."
>    (let ((mean 0)
> -        (var 0))
> +        (var 0)
> +        (hms-flag nil)
> +        (duration-flag nil))
>      (dolist (e estimates)
> -      (pcase (mapcar #'string-to-number (split-string e "-"))
> -(`(,low ,high)
> - (let ((m (/ (+ low high) 2.0)))
> -   (cl-incf mean m)
> -   (cl-incf var (- (/ (+ (* low low) (* high high)) 2.0) (* m m)))))
> -(`(,value) (cl-incf mean value))))
> -    (let ((sd (sqrt var)))
> -      (format "%s-%s"
> -      (format (or printf "%.0f") (- mean sd))
> -      (format (or printf "%.0f") (+ mean sd))))))
> +      (pcase (split-string e "-")
> +        (`(,low ,high)
> +         (dolist (time (list high low))
> +           (cond
> +            (duration-flag)
> +            ((string-match-p org-columns--duration-re time)
> +             (setq duration-flag t))
> +            (hms-flag)
> +            ((string-match-p "\\`[0-9]+:[0-9]+:[0-9]+\\'" time)
> +             (setq hms-flag t))))
> +         (let* ((low-sec (org-columns--time-to-seconds low))
> +                (high-sec (org-columns--time-to-seconds high))
> +                (m (/ (+ low-sec high-sec) 2.0)))
> +           (cl-incf mean m)
> +           (cl-incf var (- (/ (+ (* low-sec low-sec) (* high-sec
high-sec)) 2.0) (* m m)))))
> +        (`(,value) (cl-incf mean (org-columns--time-to-seconds value)))))
> +    (let* ((sd (sqrt var))
> +           (low-second (truncate (- mean sd)))
> +           (high-second (truncate (+ mean sd)))
> +           (low
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/
low-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" low-second))
> +                  (t (format-seconds "%h:%.2m" low-second))))
> +           (high
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/
high-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" high-second))
> +                  (t (format-seconds "%h:%.2m" high-second)))))
> +      (format "%s-%s" low high))))

--
Nick

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support time units in est+
  2017-01-19  4:49       ` Malcolm Matalka
@ 2017-01-19 15:44         ` Nick Dokos
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Dokos @ 2017-01-19 15:44 UTC (permalink / raw)
  To: emacs-orgmode

Malcolm Matalka <mmatalka@gmail.com> writes:

> Den 18 jan. 2017 22:19 skrev "Nick Dokos" <ndokos@gmail.com>:
>
>     Malcolm Matalka <mmatalka@gmail.com> writes:
>    
>     > Hey, this is my first elisp programming so I'm quite certain this is a
>     > big hack.  I just stole elements from elsewhere in the file.  I'm hoping
>     > this is good enough to get accepted then perhaps someone with more taste
>     > would be able to refactor it to be a bit better.
>     >
>     > Let me know if I need to change anything.
>     >
>    
>     Yes, indeed: the first thing you should do is explain what you are
>     trying to do.  E.g. what does "est+" mean? Once you have explained
>     *what* you are trying to do, then somebody might be able to suggest
>     *how* to do it better (if it is a worthwhile thing to do in the first place).
>
> I'm not sure I understand your question.  est+ is an existing functionality in columnview that is
> documented. I've brought it closer to the regular effort functionality by supporting time units.
>

My apologies: I didn't know about it.

-- 
Nick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support time units in est+
  2017-01-18 20:33 [PATCH] Support time units in est+ Malcolm Matalka
  2017-01-18 21:13 ` Nick Dokos
@ 2017-02-03 12:56 ` Malcolm Matalka
  2017-02-04 14:30 ` Nicolas Goaziou
  2 siblings, 0 replies; 6+ messages in thread
From: Malcolm Matalka @ 2017-02-03 12:56 UTC (permalink / raw)
  To: emacs-orgmode

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

Hello, has anyone had an opportunity to review this?  Thank you.

Den 18 jan. 2017 9:33 em skrev "Malcolm Matalka" <mmatalka@gmail.com>:

> Hey, this is my first elisp programming so I'm quite certain this is a
> big hack.  I just stole elements from elsewhere in the file.  I'm hoping
> this is good enough to get accepted then perhaps someone with more taste
> would be able to refactor it to be a bit better.
>
> Let me know if I need to change anything.
>
> From 1167bd20e042ad2ae3f2712f596d76ad8b230336 Mon Sep 17 00:00:00 2001
> From: orbitz <orbitz@gmail.com>
> Date: Wed, 18 Jan 2017 21:18:23 +0100
> Subject: [PATCH] org-colview.el: Add support for time units to est+
>
> * lisp/org-colview.el: Add support for time units in est+.  Ranges are
> interpreted just like non-range estimates.
>
> TINYCHANGE
> ---
> lisp/org-colview.el | 44 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lisp/org-colview.el b/lisp/org-colview.el
> index 45c71a028..2a5c067ac 100644
> --- a/lisp/org-colview.el
> +++ b/lisp/org-colview.el
> @@ -1288,23 +1288,43 @@ When PRINTF is non-nil, use it to format the
> result."
>     (/ (apply #'+ (mapcar #'org-columns--age-to-seconds ages))
>        (float (length ages)))))
>
> -(defun org-columns--summary-estimate (estimates printf)
> +(defun org-columns--summary-estimate (estimates _)
>    "Combine a list of estimates, using mean and variance.
> The mean and variance of the result will be the sum of the means
>  and variances (respectively) of the individual estimates."
>    (let ((mean 0)
> -        (var 0))
> +        (var 0)
> +        (hms-flag nil)
> +        (duration-flag nil))
>      (dolist (e estimates)
> -      (pcase (mapcar #'string-to-number (split-string e "-"))
> -(`(,low ,high)
> - (let ((m (/ (+ low high) 2.0)))
> -   (cl-incf mean m)
> -   (cl-incf var (- (/ (+ (* low low) (* high high)) 2.0) (* m m)))))
> -(`(,value) (cl-incf mean value))))
> -    (let ((sd (sqrt var)))
> -      (format "%s-%s"
> -      (format (or printf "%.0f") (- mean sd))
> -      (format (or printf "%.0f") (+ mean sd))))))
> +      (pcase (split-string e "-")
> +        (`(,low ,high)
> +         (dolist (time (list high low))
> +           (cond
> +            (duration-flag)
> +            ((string-match-p org-columns--duration-re time)
> +             (setq duration-flag t))
> +            (hms-flag)
> +            ((string-match-p "\\`[0-9]+:[0-9]+:[0-9]+\\'" time)
> +             (setq hms-flag t))))
> +         (let* ((low-sec (org-columns--time-to-seconds low))
> +                (high-sec (org-columns--time-to-seconds high))
> +                (m (/ (+ low-sec high-sec) 2.0)))
> +           (cl-incf mean m)
> +           (cl-incf var (- (/ (+ (* low-sec low-sec) (* high-sec
> high-sec)) 2.0) (* m m)))))
> +        (`(,value) (cl-incf mean (org-columns--time-to-seconds value)))))
> +    (let* ((sd (sqrt var))
> +           (low-second (truncate (- mean sd)))
> +           (high-second (truncate (+ mean sd)))
> +           (low
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/
> low-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" low-second))
> +                  (t (format-seconds "%h:%.2m" low-second))))
> +           (high
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/
> high-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" high-second))
> +                  (t (format-seconds "%h:%.2m" high-second)))))
> +      (format "%s-%s" low high))))
>
>
>
> --
> 2.11.0
>
>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support time units in est+
  2017-01-18 20:33 [PATCH] Support time units in est+ Malcolm Matalka
  2017-01-18 21:13 ` Nick Dokos
  2017-02-03 12:56 ` Malcolm Matalka
@ 2017-02-04 14:30 ` Nicolas Goaziou
  2 siblings, 0 replies; 6+ messages in thread
From: Nicolas Goaziou @ 2017-02-04 14:30 UTC (permalink / raw)
  To: Malcolm Matalka; +Cc: emacs-orgmode

Hello,

Malcolm Matalka <mmatalka@gmail.com> writes:

> Hey, this is my first elisp programming so I'm quite certain this is a
> big hack.  I just stole elements from elsewhere in the file.  I'm hoping
> this is good enough to get accepted then perhaps someone with more taste
> would be able to refactor it to be a bit better.

That's not bad. Thank you.

> Let me know if I need to change anything.
>
> From 1167bd20e042ad2ae3f2712f596d76ad8b230336 Mon Sep 17 00:00:00 2001
> From: orbitz <orbitz@gmail.com>
> Date: Wed, 18 Jan 2017 21:18:23 +0100
> Subject: [PATCH] org-colview.el: Add support for time units to est+
>
> * lisp/org-colview.el: Add support for time units in est+.  Ranges are interpreted just like non-range estimates.

You need to provide the name of the function being modified:

  * lisp/org-colview.el (org-columns--summary-estimate): Add support...
>
> TINYCHANGE
> ---
> lisp/org-colview.el | 44 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lisp/org-colview.el b/lisp/org-colview.el
> index 45c71a028..2a5c067ac 100644
> --- a/lisp/org-colview.el
> +++ b/lisp/org-colview.el
> @@ -1288,23 +1288,43 @@ When PRINTF is non-nil, use it to format the result."
>     (/ (apply #'+ (mapcar #'org-columns--age-to-seconds ages))
>        (float (length ages)))))
>  
> -(defun org-columns--summary-estimate (estimates printf)
> +(defun org-columns--summary-estimate (estimates _)
>    "Combine a list of estimates, using mean and variance.
> The mean and variance of the result will be the sum of the means
>  and variances (respectively) of the individual estimates."

Could you expound the docstring? In particular, it should make the
output format predictable. See `org-columns--summary-apply-times' for
example.

>    (let ((mean 0)
> -        (var 0))
> +        (var 0)
> +        (hms-flag nil)
> +        (duration-flag nil))
>      (dolist (e estimates)
> -      (pcase (mapcar #'string-to-number (split-string e "-"))
> -(`(,low ,high)
> - (let ((m (/ (+ low high) 2.0)))
> -   (cl-incf mean m)
> -   (cl-incf var (- (/ (+ (* low low) (* high high)) 2.0) (* m m)))))
> -(`(,value) (cl-incf mean value))))
> -    (let ((sd (sqrt var)))
> -      (format "%s-%s"
> -      (format (or printf "%.0f") (- mean sd))
> -      (format (or printf "%.0f") (+ mean sd))))))
> +      (pcase (split-string e "-")
> +        (`(,low ,high)
> +         (dolist (time (list high low))
> +           (cond
> +            (duration-flag)
> +            ((string-match-p org-columns--duration-re time)
> +             (setq duration-flag t))
> +            (hms-flag)
> +            ((string-match-p "\\`[0-9]+:[0-9]+:[0-9]+\\'" time)
> +             (setq hms-flag t))))
> +         (let* ((low-sec (org-columns--time-to-seconds low))
> +                (high-sec (org-columns--time-to-seconds high))
> +                (m (/ (+ low-sec high-sec) 2.0)))
> +           (cl-incf mean m)
> +           (cl-incf var (- (/ (+ (* low-sec low-sec) (* high-sec high-sec)) 2.0) (* m m)))))
> +        (`(,value) (cl-incf mean (org-columns--time-to-seconds
> value)))))

There is much code duplication with `org-columns--summary-apply-times'.
It would be better to avoid it.

We may want to create a new function turning a list of durations into
numbers (with `org-columns--time-to-seconds) and adding the type of the
expected output, as a symbol, in front of the list. Then both
`org-columns--summary-estimate' and `org-columns--summary-apply-times'
could use it.

It would mean we would process the list of estimates two times, but
I don't think it matters much in practice.

If you have an implementation for that, feel free to include it. Do not
consider it a blocker, tho. We can always factor this out later.

> +    (let* ((sd (sqrt var))
> +           (low-second (truncate (- mean sd)))
> +           (high-second (truncate (+ mean sd)))
> +           (low
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/ low-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" low-second))
> +                  (t (format-seconds "%h:%.2m" low-second))))
> +           (high
> +            (cond (duration-flag (org-minutes-to-clocksum-string (/  high-second 60.0)))
> +                  (hms-flag (format-seconds "%h:%.2m:%.2s" high-second))
> +                  (t (format-seconds "%h:%.2m" high-second)))))
> +      (format "%s-%s" low high))))

You don't need to `truncate' the estimates.

I don't think the default output format should be "%h:%.2m". If no
special format is present, I think the traditional "%.0f" can be used.

Also, the duplicate `cond' can be factored out. The whole part above can
be written (including suggestions aforementioned):

    (mapconcat (lambda (time)
                 (cond
                  (duration-flag (org-minutes-to-clocksum-string (/ time 60)))
                  (hms-flag (format-seconds "%h:%.2m:%.2s" time))
                  (t (format "%.0f" time))))
               (let ((sd (sqrt var))) (list (- mean sd) (+ mean sd)))
               "-")

It would be nice to add a bunch of tests for various estimates formats.
`test-org-colview/columns-summary' in "test-org-colview.el" contains
already many examples to boot.

Eventually, could you add an entry about this new feature in ORG-NEWS?


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-04 14:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-18 20:33 [PATCH] Support time units in est+ Malcolm Matalka
2017-01-18 21:13 ` Nick Dokos
     [not found]   ` <CAKziXDUDhHJWmfm2kN+tM2WmXPiqEXTfZtQYVriCBydRqiiW7g@mail.gmail.com>
     [not found]     ` <CAKziXDX_3=UTBLM8v=HZJSCR2p6tqiKwSgF+a-EXvRhsp+tAjw@mail.gmail.com>
2017-01-19  4:49       ` Malcolm Matalka
2017-01-19 15:44         ` Nick Dokos
2017-02-03 12:56 ` Malcolm Matalka
2017-02-04 14:30 ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).