emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] org-plot abstractions and extension


From: Kyle Meyer
Subject: Re: [PATCH] org-plot abstractions and extension
Date: Wed, 23 Dec 2020 07:14:31 GMT

> Subject: [PATCH] org-plot.el: fix compiler warnings
>
> * (org--plot/values-stats): Replace `log10' with `log'.
> (org--plot/nice-frequency-pick): Replace obsolete `case' with `pcase`.

case is still available under the cl- prefix.  If you wanted to use it
in 73c99bf42 (org-plot.el: add utility functions for range,ticks), I
don't see a reason not to use it now.

> (org--plot/radar): Replace `s-join' with `mapconcat', removing the
> implicit dependency on s.el.
> (org-plot/gnuplot-script): Remove unused let bindings.
> (org-plot/gnuplot-script): Replace free variable refence with expression

s/refence/reference/

> @@ -210,9 +210,9 @@ values, namely regarding the range."
>    "From a the values in a TABLE of data, attempt to guess an appropriate 
> number of ticks."
>    (let* ((row-data
>         (mapcar (lambda (row) (org--plot/values-stats
> -                         (mapcar #'string-to-number (cdr row))
> -                         hard-min
> -                         hard-max)) table))
> +                              (mapcar #'string-to-number (cdr row))
> +                              hard-min
> +                              hard-max)) table))

Please drop this unrelated space change.

> @@ -473,34 +473,36 @@ EOD
>  
>  (defun org--plot/radar (table params)
>    (let* ((data
> -       (concat "\"" (s-join "\" \"" (plist-get params :labels)) "\""
> +       (concat "\"" (mapconcat #'identity (plist-get params :labels) "\" 
> \"") "\""
>                 "\n"
> -               (s-join "\n"
> -                       (mapcar (lambda (row)
> -                                 (format
> -                                  "\"%s\" %s"
> -                                  (car row)
> -                                  (s-join " " (cdr row))))
> -                               (append table (list (car table)))))))
> +               (mapconcat #'identity
> +                          (mapcar (lambda (row)
> +                                    (format
> +                                     "\"%s\" %s"
> +                                     (car row)
> +                                     (mapconcat #'identity (cdr row) " ")))
> +                                  (append table (list (car table))))
> +                          "\n")))

The mapcar is unnecessary; you can reposition (lambda ...) as
mapconcat's FUNCTION argument.

Same comment applies to a spot farther down in the patch.

Thanks.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]