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: Thu, 24 Dec 2020 02:19:32 +0800 [thread overview]
Message-ID: <87v9cstlze.fsf@gmail.com> (raw)
In-Reply-To: <871rfgh36z.fsf@kyleam.com>
[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]
Kyle Meyer <kyle@kyleam.com> writes:
> Regardless of what you tend to use, you used "case" here in 73c99bf42;
> the minimal fix is to add a cl- prefix, and any other switch with the
> justification that "case is obsolete" is likely to raise a reviewer's
> eyebrow.
This makes sense.
> cl-case isn't in cl-lib, and there is no need to load anything.
Huh, interesting.
> recent org-plot example from 8d5122fc5:
> [...]
> That could be rewritten as [...]
Would you like me to bundle that change in somewhere?
>>>> @@ -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.
>
> This change is in org--plot/sensible-tick-num. I don't spot any
> non-whitespace changes there. Git appears to agree with me:
>
> $ git show | grep '@@ -210,9'
> @@ -210,9 +210,9 @@ (defun org--plot/sensible-tick-num (table &optional hard-min hard-max)
> $ git show -w | grep '@@ -210,9'
Ooops, I thought you were referring to one of the other regions (I saw
the "let" and "mapcar" and my brain pattern-matched the rest :P)
One question, I saw Bastien say that we didn't want whitespace-only
commits, so how should whitespace-fixups be done?
>> Subject: [PATCH] org-plot.el: fix compiler warnings
>>
>> * (org--plot/values-stats): Replace `log10' with `log'.
>
> Please add a file name ("lisp/org-plot.el") to the start of the
> changelog entry.
Ah, forgot I needed that. Sorted :)
(final?) patch revision attached.
--
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: 6785 bytes --]
From c4c7b835f27b65111859d030af58a8317a82b0a0 Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Thu, 24 Dec 2020 02:26:17 +0800
Subject: [PATCH] org-plot.el: fix compiler warnings
* lisp/org-plot.el (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 | 109 ++++++++++++++++++++++-------------------------
1 file changed, 52 insertions(+), 57 deletions(-)
diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index 4aa8276..5c6c834 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)))
@@ -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))))
+ (cl-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))))
(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
next prev parent reply other threads:[~2020-12-23 18:31 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
2020-12-23 16:55 ` Kyle Meyer
2020-12-23 18:19 ` TEC [this message]
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=87v9cstlze.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).