emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: TEC <tecosaur@gmail.com>
To: Kyle Meyer <kyle@kyleam.com>
Cc: Bastien <bzg@gnu.org>, org-mode-email <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] org-plot abstractions and extension
Date: Wed, 23 Dec 2020 16:38:24 +0800	[thread overview]
Message-ID: <87wnx8ud5k.fsf@gmail.com> (raw)
In-Reply-To: <87czz1gfje.fsf@kyleam.com>

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


Kyle Meyer <kyle@kyleam.com> writes:

> 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.

I tend to use pcase over cl-case (since it's completely built in, i.e.
no (require 'cl-lib) required). I'm not sure if there's any argument for
cl-case over pcase, let me know if so.

> s/refence/reference/

Done

>> @@ -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.

Erm, this isn't unrelated. As the function being called changed length,
the indentation of the arguments is thus also changed.

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

Thanks for spotting that. Resolved.

Updated patch attached. Let me know how it looks to you :)

--
Timothy


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-plot.el-fix-compiler-warnings.patch --]
[-- Type: text/x-patch, Size: 7285 bytes --]

From 22717d0750e2c001003b45f1d4834571f21287ef Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Wed, 23 Dec 2020 14:13:24 +0800
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`.
(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 reference with expression
only using given variables.
---
 lisp/org-plot.el | 115 +++++++++++++++++++++++------------------------
 1 file changed, 55 insertions(+), 60 deletions(-)

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index 4aa8276..1c7ee43 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -196,7 +196,7 @@ values, namely regarding the range."
 	 (maximum (or hard-max (apply #'max nums)))
 	 (range (- maximum minimum))
 	 (rangeOrder (if (= range 0) 0
-		       (ceiling (- 1 (log10 range)))))
+		       (ceiling (- 1 (log range 10)))))
 	 (range-factor (expt 10 rangeOrder))
 	 (nice-min (if (= range 0) (car nums)
 		     (/ (float (floor (* minimum range-factor))) range-factor)))
@@ -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))
 	 (row-normalised-ranges (mapcar (lambda (r-data)
 					  (let ((val (round (*
 							     (plist-get r-data :range-factor)
@@ -229,34 +229,34 @@ values, namely regarding the range."
 (defun org--plot/nice-frequency-pick (frequencies)
   "From a list of frequences, try to sensibly pick a sample of the most frequent."
   ;; TODO this mosly works decently, but counld do with some tweaking to work more consistently.
-  (case (length frequencies)
-	(1 (list (car (nth 0 frequencies))))
-	(2 (if (<= 3 (/ (cdr (nth 0 frequencies))
-			(cdr (nth 1 frequencies))))
-	       (make-list 2
-			  (car (nth 0 frequencies)))
-	     (list (car (nth 0 frequencies))
-		   (car (nth 1 frequencies)))))
-	(t
-	 (let* ((total-count (apply #'+ (mapcar #'cdr frequencies)))
-		(n-freq (mapcar (lambda (freq) `(,(car freq) . ,(/ (float (cdr freq)) total-count))) frequencies))
-		(f-pick (list (car (car n-freq))))
-		(1-2-ratio (/ (cdr (nth 0 n-freq))
-			      (cdr (nth 1 n-freq))))
-		(2-3-ratio (/ (cdr (nth 1 n-freq))
-			      (cdr (nth 2 n-freq))))
-		(1-3-ratio (* 1-2-ratio 2-3-ratio))
-		(1-val (car (nth 0 n-freq)))
-		(2-val (car (nth 1 n-freq)))
-		(3-val (car (nth 2 n-freq))))
-	   (when (> 1-2-ratio 4) (push 1-val f-pick))
-	   (when (and (< 1-2-ratio 2-val)
-		      (< (* (apply #'* f-pick) 2-val) 30))
-	     (push 2-val f-pick))
-	   (when (and (< 1-3-ratio 3-val)
-		      (< (* (apply #'* f-pick) 3-val) 30))
-	     (push 3-val f-pick))
-	   f-pick))))
+  (pcase (length frequencies)
+    (1 (list (car (nth 0 frequencies))))
+    (2 (if (<= 3 (/ (cdr (nth 0 frequencies))
+		    (cdr (nth 1 frequencies))))
+	   (make-list 2
+		      (car (nth 0 frequencies)))
+	 (list (car (nth 0 frequencies))
+	       (car (nth 1 frequencies)))))
+    (_
+     (let* ((total-count (apply #'+ (mapcar #'cdr frequencies)))
+	    (n-freq (mapcar (lambda (freq) `(,(car freq) . ,(/ (float (cdr freq)) total-count))) frequencies))
+	    (f-pick (list (car (car n-freq))))
+	    (1-2-ratio (/ (cdr (nth 0 n-freq))
+			  (cdr (nth 1 n-freq))))
+	    (2-3-ratio (/ (cdr (nth 1 n-freq))
+			  (cdr (nth 2 n-freq))))
+	    (1-3-ratio (* 1-2-ratio 2-3-ratio))
+	    (1-val (car (nth 0 n-freq)))
+	    (2-val (car (nth 1 n-freq)))
+	    (3-val (car (nth 2 n-freq))))
+       (when (> 1-2-ratio 4) (push 1-val f-pick))
+       (when (and (< 1-2-ratio 2-val)
+		  (< (* (apply #'* f-pick) 2-val) 30))
+	 (push 2-val f-pick))
+       (when (and (< 1-3-ratio 3-val)
+		  (< (* (apply #'* f-pick) 3-val) 30))
+	 (push 3-val f-pick))
+       f-pick))))
 
 (defun org--plot/merge-alists (function default alist1 alist2 &rest alists)
   "Using FUNCTION, combine the elements of all given ALISTS. When an element is
@@ -473,34 +473,34 @@ 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 (lambda (row)
+			       (format
+				"\"%s\" %s"
+				(car row)
+				(mapconcat #'identity (cdr row) " ")))
+			     (append table (list (car table)))
+			     "\n")))
 	 (ticks (or (plist-get params :ticks)
 		    (org--plot/sensible-tick-num table
 						 (plist-get params :ymin)
 						 (plist-get params :ymax))))
 	 (settings
-	  (s-join "\n"
-		  (mapcar (lambda (row)
-			    (let ((data (org--plot/values-stats
-					 (mapcar #'string-to-number (cdr row)))))
-			      (format
-			       "\"%s\" %s %s %s"
-			       (car row)
-			       (or (plist-get params :ymin)
-				   (plist-get data :nice-min))
-			       (or (plist-get params :ymax)
-				   (plist-get data :nice-max))
-			       (if (eq ticks 0) 2 ticks)
-			       )))
-			  (append table (list (car table))))))
+	  (mapconcat (lambda (row)
+		       (let ((data (org--plot/values-stats
+				    (mapcar #'string-to-number (cdr row)))))
+			 (format
+			  "\"%s\" %s %s %s"
+			  (car row)
+			  (or (plist-get params :ymin)
+			      (plist-get data :nice-min))
+			  (or (plist-get params :ymax)
+			      (plist-get data :nice-max))
+			  (if (eq ticks 0) 2 ticks)
+			  )))
+		     (append table (list (car table)))
+		     "\n"))
 	 (setup-file (make-temp-file "org-plot-setup")))
     (let ((coding-system-for-write 'utf-8))
       (write-region (format org--plot/radar-setup-template data settings) nil setup-file nil :silent))
@@ -533,15 +533,10 @@ manner suitable for prepending to a user-specified script."
       (user-error "Org-plot type `%s' is undefined." type-name))
     (let* ((sets (plist-get params :set))
 	   (lines (plist-get params :line))
-	   (map (plist-get params :map))
 	   (title (plist-get params :title))
 	   (file (plist-get params :file))
-	   (ind (plist-get params :ind))
 	   (time-ind (plist-get params :timeind))
 	   (timefmt (plist-get params :timefmt))
-	   (text-ind (plist-get params :textind))
-	   (deps (if (plist-member params :deps) (plist-get params :deps)))
-	   (col-labels (plist-get params :labels))
 	   (x-labels (plist-get params :xlabels))
 	   (y-labels (plist-get params :ylabels))
 	   (plot-str (or (plist-get type :plot-str)
@@ -650,7 +645,7 @@ line directly before or after the table."
 			org-plot/preset-plot-types)))
 
       (unless type
-	(user-error "Org-plot type `%s' is undefined." type-name))
+	(user-error "Org-plot type `%s' is undefined." (plist-get params :plot-type)))
 
       (run-with-idle-timer 0.1 nil #'delete-file data-file)
       (when (eq (cadr table) 'hline)
-- 
2.29.2


  reply	other threads:[~2020-12-23  8:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-06  5:50 [PATCH] org-plot abstractions and extension TEC
     [not found] ` <87blijmnv9.fsf@gnu.org>
     [not found]   ` <CAHNg_jM8sE4a6XvL5D8Gks4dQXfWhZvRBR33BDLkRgEgZ++ZGg@mail.gmail.com>
2020-09-15  3:43     ` TEC
2020-09-25 17:51       ` TEC
2020-10-17  2:12         ` TEC
2020-10-24 11:31           ` Bastien
2020-10-24 18:16             ` TEC
2020-11-21 11:49               ` ian martins
2020-12-09  2:58               ` TEC
2020-12-10 10:28                 ` Bastien
2020-12-14  5:41               ` Bastien
2020-12-14  6:30                 ` TEC
2020-12-14  6:56                   ` Bastien
2020-12-23  5:09                 ` Kyle Meyer
2020-12-23  5:10                   ` TEC
2020-12-23  6:19                     ` TEC
2020-12-23  7:14                       ` Kyle Meyer
2020-12-23  8:38                         ` TEC [this message]
2020-12-23 16:55                           ` Kyle Meyer
2020-12-23 18:19                             ` TEC
2020-12-24  3:09                               ` Kyle Meyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wnx8ud5k.fsf@gmail.com \
    --to=tecosaur@gmail.com \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=kyle@kyleam.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).