`with` as a list.

`with` as a list.

[Mario Frasca]

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

good day to you all

now and then I use emacs to make graphs.  now recently I was plotting 
point data, and a running average "fit", so I wanted to have points, and 
lines, which I know it's possible in `gnuplot` but now how do I do that 
from org-plot …

I wrote a small patch for org-plot.el, I'm not a Lisp programmer so I'm 
sure the patch looks terrible, but it does allow me to do this:

#+PLOT: ind:1 deps:(3 6 4 7) with:(points lines points lines)

it's two additions:

1. it lets me specify the order in which the dependent columns should be 
considered.

2. it lets me specify a different `with` for each column, in the same order.

if you leave the `with` away, you get "lines" for all columns.

if you specify only one `with` value, that value is used for all columns.

if you specify more `deps` than `with`, the ones not specified will get 
"lines".

if you specify more `with` than `deps`, they are ignored.

I ran the tests, and I get two failing ones, quite unrelated according 
to me:

2 unexpected results:
    FAILED  ob-exp/evaluate-all-executables-in-order
    FAILED  ob-exp/export-call-line-information

I have not defined test cases for the new behaviour, I'm willing to do 
that (learning the way this test environment works), but I don't find 
the location of the other tests related to the area of the program, 
which I'm tweaking.

best regards all,

Mario Frasca


[-- Attachment #2: with-as-a-list.patch --]
[-- Type: text/x-patch, Size: 3109 bytes --]

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index a23195d2a..87a415137 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -179,6 +179,28 @@ and dependent variables."
 	  (setf back-edge "") (setf front-edge ""))))
     row-vals))
 
+(defun org-plot/zip-deps-with (num-cols ind deps with)
+  "describe each column to be plotted as (col . with)"
+  ;; make 'deps explicit
+  (unless deps
+    (setf deps (let (r)
+		 (dotimes (i num-cols r)
+		   (unless (eq num-cols (+ ind i))
+		     (setq r (cons (- num-cols i) r)))))))
+  ;; make sure 'with matches 'deps
+  (unless with
+    (setf with "lines"))
+  (unless (listp with)
+    (setf with (mapcar (lambda (x) with) deps)))
+  ;; invoke zipping function on converted data
+  (org-plot/zip deps with))
+
+(defun org-plot/zip (xs ys)
+  (unless
+      (null xs)
+    (cons (cons (car xs) (or (car ys) "lines"))
+	  (org-plot/zip (cdr xs) (cdr ys)))))
+
 (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
   "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
 NUM-COLS controls the number of columns plotted in a 2-d plot.
@@ -240,22 +262,22 @@ manner suitable for prepending to a user-specified script."
 			       "%Y-%m-%d-%H:%M:%S") "\"")))
     (unless preface
       (pcase type			; plot command
-	(`2d (dotimes (col num-cols)
-	       (unless (and (eq type '2d)
-			    (or (and ind (equal (1+ col) ind))
-				(and deps (not (member (1+ col) deps)))))
-		 (setf plot-lines
-		       (cons
-			(format plot-str data-file
-				(or (and ind (> ind 0)
-					 (not text-ind)
-					 (format "%d:" ind)) "")
-				(1+ col)
-				(if text-ind (format ":xticlabel(%d)" ind) "")
-				with
-				(or (nth col col-labels)
-				    (format "%d" (1+ col))))
-			plot-lines)))))
+	(`2d (dolist
+		 (col-with
+		  (org-plot/zip-deps-with num-cols ind deps with))
+	       (setf plot-lines
+		     (cons
+		      (format plot-str data-file
+			      (or (and ind (> ind 0)
+				       (not text-ind)
+				       (format "%d:" ind)) "")
+			      (car col-with)
+			      (if text-ind (format ":xticlabel(%d)" ind) "")
+			      (cdr col-with)
+			      (or (nth (1- (car col-with))
+				       col-labels)
+				  (format "%d" (car col-with))))
+		      plot-lines))))
 	(`3d
 	 (setq plot-lines (list (format "'%s' matrix with %s title ''"
 					data-file with))))
@@ -310,7 +332,8 @@ line directly before or after the table."
 				table data-file params)))
 		 (when y-labels (plist-put params :ylabels y-labels)))))
       ;; Check for timestamp ind column.
-      (let ((ind (1- (plist-get params :ind))))
+      (let ((ind (1- (plist-get params :ind)))
+	    (with (plist-get params :with)))
 	(when (and (>= ind 0) (eq '2d (plist-get params :plot-type)))
 	  (if (= (length
 		  (delq 0 (mapcar
@@ -320,7 +343,7 @@ line directly before or after the table."
 		 0)
 	      (plist-put params :timeind t)
 	    ;; Check for text ind column.
-	    (if (or (string= (plist-get params :with) "hist")
+	    (if (or (and (stringp with) (string= with "hist"))
 		    (> (length
 			(delq 0 (mapcar
 				 (lambda (el)

[PATCH] [FEATURE] Re: `with` as a list.

[Mario Frasca]

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

I have added a couple of unit tests to the suite, describing the two 
functions I added.  I have no unexpectedly failing tests now.

I'm explicitly cc-ing Eric Schulte because he's in the header for 
org-plot.el, and —missing the unit tests for that source— I hope he can 
assist me not breaking what he wrote.  … I guess I can write also test 
cases for what already exists, but I will very likely need assistance.

best regards,
Mario Frasca

On 22/05/2020 11:07, Mario Frasca wrote:
> good day to you all
>
> now and then I use emacs to make graphs.  now recently I was plotting 
> point data, and a running average "fit", so I wanted to have points, 
> and lines, which I know it's possible in `gnuplot` but now how do I do 
> that from org-plot …
>
> I wrote a small patch for org-plot.el, I'm not a Lisp programmer so 
> I'm sure the patch looks terrible, but it does allow me to do this:
>
> #+PLOT: ind:1 deps:(3 6 4 7) with:(points lines points lines)
>
> it's two additions:
>
> 1. it lets me specify the order in which the dependent columns should 
> be considered.
>
> 2. it lets me specify a different `with` for each column, in the same 
> order.
>
> if you leave the `with` away, you get "lines" for all columns.
>
> if you specify only one `with` value, that value is used for all columns.
>
> if you specify more `deps` than `with`, the ones not specified will 
> get "lines".
>
> if you specify more `with` than `deps`, they are ignored.
>
> I ran the tests, and I get two failing ones, quite unrelated according 
> to me:
>
> 2 unexpected results:
>    FAILED  ob-exp/evaluate-all-executables-in-order
>    FAILED  ob-exp/export-call-line-information
>
> I have not defined test cases for the new behaviour, I'm willing to do 
> that (learning the way this test environment works), but I don't find 
> the location of the other tests related to the area of the program, 
> which I'm tweaking.
>
> best regards all,
>
> Mario Frasca
>

[-- Attachment #2: plot-with-list.patch --]
[-- Type: text/x-patch, Size: 5263 bytes --]

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index a23195d2a..524615d98 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -179,6 +179,28 @@ and dependent variables."
 	  (setf back-edge "") (setf front-edge ""))))
     row-vals))
 
+(defun org-plot/zip-deps-with (num-cols ind deps with)
+  "describe each column to be plotted as (col . with)"
+  ;; make 'deps explicit
+  (unless deps
+    (setf deps (let (r)
+		 (dotimes (i num-cols r)
+		   (unless (eq num-cols (+ ind i))
+		     (setq r (cons (- num-cols i) r)))))))
+  ;; make sure 'with matches 'deps
+  (unless with
+    (setf with "lines"))
+  (unless (listp with)
+    (setf with (make-list (length deps) with)))
+  ;; invoke zipping function on converted data
+  (org-plot/zip deps with))
+
+(defun org-plot/zip (xs ys)
+  (unless
+      (null xs)
+    (cons (cons (car xs) (or (car ys) "lines"))
+	  (org-plot/zip (cdr xs) (cdr ys)))))
+
 (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
   "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
 NUM-COLS controls the number of columns plotted in a 2-d plot.
@@ -240,22 +262,24 @@ manner suitable for prepending to a user-specified script."
 			       "%Y-%m-%d-%H:%M:%S") "\"")))
     (unless preface
       (pcase type			; plot command
-	(`2d (dotimes (col num-cols)
-	       (unless (and (eq type '2d)
-			    (or (and ind (equal (1+ col) ind))
-				(and deps (not (member (1+ col) deps)))))
-		 (setf plot-lines
-		       (cons
-			(format plot-str data-file
-				(or (and ind (> ind 0)
-					 (not text-ind)
-					 (format "%d:" ind)) "")
-				(1+ col)
-				(if text-ind (format ":xticlabel(%d)" ind) "")
-				with
-				(or (nth col col-labels)
-				    (format "%d" (1+ col))))
-			plot-lines)))))
+	(`2d (dolist
+		 (col-with
+		  (org-plot/zip-deps-with num-cols ind deps with))
+	       (setf plot-lines
+		     (cons
+		      (format plot-str data-file
+			      (or (and ind (> ind 0)
+				       (not text-ind)
+				       (format "%d:" ind)) "")
+			      (car col-with)
+			      (if text-ind (format ":xticlabel(%d)" ind) "")
+			      (cdr col-with)
+			      (apply (lambda (x)
+				       (if (= 0 (length x))
+					   (format "%d" (car col-with))
+					 x))
+				     (list (nth (1- (car col-with)) col-labels))))
+		      plot-lines))))
 	(`3d
 	 (setq plot-lines (list (format "'%s' matrix with %s title ''"
 					data-file with))))
@@ -310,7 +334,8 @@ line directly before or after the table."
 				table data-file params)))
 		 (when y-labels (plist-put params :ylabels y-labels)))))
       ;; Check for timestamp ind column.
-      (let ((ind (1- (plist-get params :ind))))
+      (let ((ind (1- (plist-get params :ind)))
+	    (with (plist-get params :with)))
 	(when (and (>= ind 0) (eq '2d (plist-get params :plot-type)))
 	  (if (= (length
 		  (delq 0 (mapcar
@@ -320,7 +345,7 @@ line directly before or after the table."
 		 0)
 	      (plist-put params :timeind t)
 	    ;; Check for text ind column.
-	    (if (or (string= (plist-get params :with) "hist")
+	    (if (or (and (stringp with) (string= with "hist"))
 		    (> (length
 			(delq 0 (mapcar
 				 (lambda (el)
diff --git a/testing/lisp/test-org-plot.el b/testing/lisp/test-org-plot.el
new file mode 100644
index 000000000..4ed8c15b4
--- /dev/null
+++ b/testing/lisp/test-org-plot.el
@@ -0,0 +1,62 @@
+;;; test-org-plot.el --- Tests for org-plot.el
+
+;; Copyright (C) 2020  Mario Frasca
+
+;; Author: Mario Frasca <mario at anche dot no>
+
+;; Released under the GNU General Public License version 3
+;; see: http://www.gnu.org/licenses/gpl-3.0.html
+
+;;;; Comments
+
+
+\f
+;;; Code:
+
+(require 'org-plot)
+
+(ert-deftest test-org-plot/zip ()
+  "Test `org-plot/zip' specifications."
+  ;; zipping two equal length lists
+  (should
+   (equal '((1 . "a") (2 . "b") (3 . "c"))
+	  (org-plot/zip '(1 2 3) '("a" "b" "c"))))
+  ;; if second is shorter, fill in with "lines"
+  (should
+   (equal '((1 . "a") (2 . "b") (3 . "lines"))
+	  (org-plot/zip '(1 2 3) '("a" "b"))))
+  ;; if first is shorter, stop there
+  (should
+   (equal '((1 . "a") (2 . "b"))
+	  (org-plot/zip '(1 2) '("a" "b" "c")))))
+
+(ert-deftest test-org-plot/zip-deps-with ()
+  "Test `org-plot/zip-deps-with' specifications."
+  ;; no deps, no with. defaults to all except ind, and "lines"
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil nil)
+	  '((2 . "lines") (3 . "lines"))))
+  ;; no deps, single with. defaults to all except ind, and repeated with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil "hist")
+	  '((2 . "hist") (3 . "hist"))))
+  ;; no deps, explicit with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil '("points" "hist"))
+	  '((2 . "points") (3 . "hist"))))
+  ;; explicit with, same length as deps
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(2 4) '("points" "hist"))
+	  '((2 . "points") (4 . "hist"))))
+  ;; same as above, but different order
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2) '("points" "hist"))
+	  '((4 . "points") (2 . "hist"))))
+  ;; fills in with "lines"
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2 3) '("points"))
+	  '((4 . "points") (2 . "lines") (3 . "lines")))))
+  
+     
+(provide 'test-org-plot)
+;;; test-org-plot.el end here

Re: `with` as a list.

[Kyle Meyer]

Mario Frasca writes:

> now and then I use emacs to make graphs.  now recently I was plotting 
> point data, and a running average "fit", so I wanted to have points, and 
> lines, which I know it's possible in `gnuplot` but now how do I do that 
> from org-plot …
>
> I wrote a small patch for org-plot.el, I'm not a Lisp programmer so I'm 
> sure the patch looks terrible, but it does allow me to do this:
>
> #+PLOT: ind:1 deps:(3 6 4 7) with:(points lines points lines)
>
> it's two additions: [...]

Thanks for the patch and for clearly describing the motivation.  I'm not
an org-plot user, but the change sounds useful.  (It'd be great if
org-plot users could chime in.)

Two meta-comments:

  * Please provide a patch with a commit message.  See
    <https://orgmode.org/worg/org-contribute.html> for more information.

  * The link above also explains the copyright assignment requirement
    for contributions.  Please consider assigning copyright to the FSF.

> diff --git a/lisp/org-plot.el b/lisp/org-plot.el
> index a23195d2a..87a415137 100644
> --- a/lisp/org-plot.el
> +++ b/lisp/org-plot.el
> @@ -179,6 +179,28 @@ and dependent variables."
>  	  (setf back-edge "") (setf front-edge ""))))
>      row-vals))
>  
> +(defun org-plot/zip-deps-with (num-cols ind deps with)
> +  "describe each column to be plotted as (col . with)"

This doesn't match the convention used in this code base for docstrings.
Taking a look around should give you a good sense, but (1) the first
word should be capitalized, (2) sentences should end with a period, and
(3) ideally all parameters should be described in the docstring.

> +  ;; make 'deps explicit

I think this and the other comments in this function could safely be
dropped.

> +  (unless deps
> +    (setf deps (let (r)
> +		 (dotimes (i num-cols r)
> +		   (unless (eq num-cols (+ ind i))
> +		     (setq r (cons (- num-cols i) r)))))))

Hmm, org-plot does seem to use setf a lot (more than any other .el file
in the repo), though using setq unless setf is needed would be more
consistent with this code base.

The code above looks fine to me.  Another option would be to use
number-sequence and then filter out the ind value.

> +  ;; make sure 'with matches 'deps
> +  (unless with
> +    (setf with "lines"))
> +  (unless (listp with)
> +    (setf with (mapcar (lambda (x) with) deps)))

This is make-list in the more recent diff you sent, which I agree is
better.

> +  ;; invoke zipping function on converted data
> +  (org-plot/zip deps with))
> +
> +(defun org-plot/zip (xs ys)
> +  (unless
> +      (null xs)
> +    (cons (cons (car xs) (or (car ys) "lines"))

Is the "lines" fall back ever reached?  It looks like you're extending
`with' above to be the same length as `deps`.

> +	  (org-plot/zip (cdr xs) (cdr ys)))))

In Elisp, it's not very common to use recursive functions (and there's
no TCO).  In the case of zipping two lists, I think it'd probably be
easiest to just use cl-loop.  And in my view having a separate function
here is probably an overkill.

>  (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
>    "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
>  NUM-COLS controls the number of columns plotted in a 2-d plot.
> @@ -240,22 +262,22 @@ manner suitable for prepending to a user-specified script."
>  			       "%Y-%m-%d-%H:%M:%S") "\"")))
>      (unless preface
>        (pcase type			; plot command
> -	(`2d (dotimes (col num-cols)
> -	       (unless (and (eq type '2d)
> -			    (or (and ind (equal (1+ col) ind))
> -				(and deps (not (member (1+ col) deps)))))
> -		 (setf plot-lines
> -		       (cons
> -			(format plot-str data-file
> -				(or (and ind (> ind 0)
> -					 (not text-ind)
> -					 (format "%d:" ind)) "")
> -				(1+ col)
> -				(if text-ind (format ":xticlabel(%d)" ind) "")
> -				with
> -				(or (nth col col-labels)
> -				    (format "%d" (1+ col))))
> -			plot-lines)))))
> +	(`2d (dolist
> +		 (col-with
> +		  (org-plot/zip-deps-with num-cols ind deps with))
> +	       (setf plot-lines
> +		     (cons
> +		      (format plot-str data-file
> +			      (or (and ind (> ind 0)
> +				       (not text-ind)
> +				       (format "%d:" ind)) "")
> +			      (car col-with)
> +			      (if text-ind (format ":xticlabel(%d)" ind) "")
> +			      (cdr col-with)
> +			      (or (nth (1- (car col-with))
> +				       col-labels)
> +				  (format "%d" (car col-with))))
> +		      plot-lines))))

I haven't looked at this bit too closely (and I know the handling around
col-labels changed a bit in the last message you sent), but I did try
out a few org-plot invocations and things seemed to work as I expected.
I'll plan to take a closer when you send a reroll.

> @@ -320,7 +343,7 @@ line directly before or after the table."
>  		 0)
>  	      (plist-put params :timeind t)
>  	    ;; Check for text ind column.
> -	    (if (or (string= (plist-get params :with) "hist")
> +	    (if (or (and (stringp with) (string= with "hist"))

Could be simplified by using `equal'.

Thanks again for the patch.

Re: `with` as a list.

[Mario Frasca]

Hi Kyle,

thank you for writing back, I have a couple of questions in reply.

btw. are you on irc.freenode.net?  I'm `mariotomo' there.  and I just 
joined `#org-mode'.  I don't think that my terminal will ring a bell if 
I'm mentioned there.

On 30/05/2020 01:22, Kyle Meyer wrote:
> Mario Frasca writes:
>
>> […]
> Thanks for the patch and for clearly describing the motivation.  I'm not
> an org-plot user, but the change sounds useful.  (It'd be great if
> org-plot users could chime in.)

thank you for taking the time to review!


> Two meta-comments:
>
>    * Please provide a patch with a commit message.  See
>      <https://orgmode.org/worg/org-contribute.html> for more information.
>
>    * The link above also explains the copyright assignment requirement
>      for contributions.  Please consider assigning copyright to the FSF.

I'm editing in my cloned repository, and committing often, so I do not 
have a single commit, consequently also not a single commit message.

I just sent my form request to assign@gnu.org.


>> diff --git a/lisp/org-plot.el b/lisp/org-plot.el
>> index a23195d2a..87a415137 100644
>> --- a/lisp/org-plot.el
>> +++ b/lisp/org-plot.el
>> @@ -179,6 +179,28 @@ and dependent variables."
>>   	  (setf back-edge "") (setf front-edge ""))))
>>       row-vals))
>>   
>> +(defun org-plot/zip-deps-with (num-cols ind deps with)
>> +  "describe each column to be plotted as (col . with)"
> This doesn't match the convention used in this code base for docstrings.
> Taking a look around should give you a good sense, but (1) the first
> word should be capitalized, (2) sentences should end with a period, and
> (3) ideally all parameters should be described in the docstring.

ok, 1 and 2 are just consequence of my usual way of typing, however 
wrong, I will fix them.  3 I didn't consider.  useful point.


>
>> +  ;; make 'deps explicit
> I think this and the other comments in this function could safely be
> dropped.

:+1:


>> +  (unless deps
>> +    (setf deps (let (r)
>> +		 (dotimes (i num-cols r)
>> +		   (unless (eq num-cols (+ ind i))
>> +		     (setq r (cons (- num-cols i) r)))))))
> […] using setq unless setf is needed would be more
> consistent with this code base.

the "unless needed" makes me suspect I should read what's the difference.


> The code above looks fine to me.  Another option would be to use
> number-sequence and then filter out the ind value.

no, that would break functionality: I need to keep the given order.

btw my patch allows you to use the same column more than once.


>
>> +  ;; invoke zipping function on converted data
>> +  (org-plot/zip deps with))
>> +
>> +(defun org-plot/zip (xs ys)
>> +  (unless
>> +      (null xs)
>> +    (cons (cons (car xs) (or (car ys) "lines"))
> Is the "lines" fall back ever reached?  It looks like you're extending
> `with' above to be the same length as `deps`.

it is needed: I'm not extending any `with' given as a non-empty list.

but I should be using some settings, some global default `with' value, 
which I don't know where to find.  hard coding "lines" can't be the 
right thing to do.

>> +	  (org-plot/zip (cdr xs) (cdr ys)))))
> In Elisp, it's not very common to use recursive functions (and there's
> no TCO).  In the case of zipping two lists, I think it'd probably be
> easiest to just use cl-loop.  And in my view having a separate function
> here is probably an overkill.

I'm not sure about the TCO (in other words: I haven't the faintest idea 
what you mean by that), and I would not know how to write this using 
`cl-loop'.  I'll have a look though.


>>   (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
>>     "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
>>   NUM-COLS controls the number of columns plotted in a 2-d plot.
>> @@ -240,22 +262,22 @@ manner suitable for prepending to a user-specified script."
>>   			       "%Y-%m-%d-%H:%M:%S") "\"")))
>>       (unless preface
>>         (pcase type			; plot command
>> -	(`2d (dotimes (col num-cols)
>> -	       (unless (and (eq type '2d)
>> -			    (or (and ind (equal (1+ col) ind))
>> -				(and deps (not (member (1+ col) deps)))))
>> -		 (setf plot-lines
>> -		       (cons
>> -			(format plot-str data-file
>> -				(or (and ind (> ind 0)
>> -					 (not text-ind)
>> -					 (format "%d:" ind)) "")
>> -				(1+ col)
>> -				(if text-ind (format ":xticlabel(%d)" ind) "")
>> -				with
>> -				(or (nth col col-labels)
>> -				    (format "%d" (1+ col))))
>> -			plot-lines)))))
>> +	(`2d (dolist
>> +		 (col-with
>> +		  (org-plot/zip-deps-with num-cols ind deps with))
>> +	       (setf plot-lines
>> +		     (cons
>> +		      (format plot-str data-file
>> +			      (or (and ind (> ind 0)
>> +				       (not text-ind)
>> +				       (format "%d:" ind)) "")
>> +			      (car col-with)
>> +			      (if text-ind (format ":xticlabel(%d)" ind) "")
>> +			      (cdr col-with)
>> +			      (or (nth (1- (car col-with))
>> +				       col-labels)
>> +				  (format "%d" (car col-with))))
>> +		      plot-lines))))
> I haven't looked at this bit too closely (and I know the handling around
> col-labels changed a bit in the last message you sent), but I did try
> out a few org-plot invocations and things seemed to work as I expected.
> I'll plan to take a closer when you send a reroll.

the whole org-plot.el has (had) no unit tests, so if you share a couple 
of your org-plot invocations, I can convert them to regression tests.


>> @@ -320,7 +343,7 @@ line directly before or after the table."
>>   		 0)
>>   	      (plist-put params :timeind t)
>>   	    ;; Check for text ind column.
>> -	    (if (or (string= (plist-get params :with) "hist")
>> +	    (if (or (and (stringp with) (string= with "hist"))
> Could be simplified by using `equal'.

yes, definitely!  as said, I'm not a lisp programmer.  ;-)

I hope to post a new diff soon.

cheers, MF

Re: `with` as a list.

[Mario Frasca]

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

On 30/05/2020 09:23, Mario Frasca wrote:
>> The code above looks fine to me.  Another option would be to use
>> number-sequence and then filter out the ind value.
>
> no, that would break functionality: I need to keep the given order.
ah, no, sorry, you are totally right here.  I'll see if using 
number-sequence makes the code shorter.

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

Re: `with` as a list.

[Kyle Meyer]

Mario Frasca writes:

> btw. are you on irc.freenode.net?  I'm `mariotomo' there.  and I just 
> joined `#org-mode'.  I don't think that my terminal will ring a bell if 
> I'm mentioned there.

No, sorry, I'm not.

> On 30/05/2020 01:22, Kyle Meyer wrote:
>> Mario Frasca writes:
>
>> Two meta-comments:
>>
>>    * Please provide a patch with a commit message.  See
>>      <https://orgmode.org/worg/org-contribute.html> for more information.
>>
>>    * The link above also explains the copyright assignment requirement
>>      for contributions.  Please consider assigning copyright to the FSF.
>
> I'm editing in my cloned repository, and committing often, so I do not 
> have a single commit, consequently also not a single commit message.

There are various ways to get those fixups into a single commit.  Here's
one that assumes that you're working from the command line and on a
topic branch rather than directly on master.  Do an interactive rebase
(`git rebase -i master').  When the editor pops up, replace "pick" with
"fixup" in all of the lines but the first.  (If any of the "fixup"
commits have message bits that you want to keep in the final message,
you can use "squash" instead.)

For later fixups, you can do the same process.  You could also use
git-commit's --amend.  (Again, there are lots of possible workflows, and
of course once you're working on a series of commits rather than a
single one, the process changes a bit.)

> I just sent my form request to assign@gnu.org.

Thank you!  Please let us know when the process is complete.

I see you already sent an updated patch, so I'll just respond to one
more point from this message.

>> I haven't looked at this bit too closely (and I know the handling around
>> col-labels changed a bit in the last message you sent), but I did try
>> out a few org-plot invocations and things seemed to work as I expected.
>> I'll plan to take a closer when you send a reroll.
>
> the whole org-plot.el has (had) no unit tests, so if you share a couple 
> of your org-plot invocations, I can convert them to regression tests.

I don't use org-plot.el, so to try this out, I just copied an example
from the Worg tutorial, stripped the table down a bit, and tried a few
`deps' and `with' values.  So there's nothing too useful, but here's is
the last one I had:

--8<---------------cut here---------------start------------->8---
#+PLOT: title:"example table" ind:1 deps:(2 3) type:2d with:(lines points)
#+PLOT: labels:("xlab" "ylab" "zlab")
#+TBLNAME:org-plot-example-1
|   x |    y |    z |
|-----+------+------|
| 0.1 | 0.42 | 0.37 |
| 0.2 | 0.31 | 0.33 |
| 0.3 | 0.24 | 0.28 |
| 0.4 | 0.27 | 0.23 |
--8<---------------cut here---------------end--------------->8---

Thanks for your interest in adding tests.  Of course it'd be good for
org-plot.el to have some, and there are existing spots where I think it
should be easy enough to hook into and test (in particular generating
the script file and data file).  However, that doesn't necessarily have
to be tacked on to this series.

Re: `with` as a list.

[Mario Frasca]

oops .. you mentioned `cl-loop' and I found it interesting, in 
particular the de-structuring part.

so I rewrote the (dolist (col-with …) …) as (cl-loop for (col . with) in 
… do …).

so I could simplify `(car col-with)' and `(cdr col-with)', then I 
replaced the `do' with a `collect', so I could squash other `setq' into 
one, and finally removed the need to do a `reverse' on the result.  less 
parentheses, and 4 lines less.

I added one regression test which is still respected, as is the rest of 
the test suite.

so maybe all is fine.

putting order in the commits is now the challenge.  thank you for your 
hints on git commit amend etc.

I hope to be back soon with a single commit...

ciao, Mario

Re: `with` as a list.

[Mario Frasca]

On 30/05/2020 16:29, Mario Frasca wrote:
> I hope to be back soon with a single commit... 

one doubt.  what's the point of having me squash all in a single commit, 
when I do not have write access to the repository?  if we were on 
github, I would be working on a pull request, which would have a 
description and a title, and contain several commits.  I don't 
understand the workflow apparently.

Re: `with` as a list.

[Kyle Meyer]

Mario Frasca writes:

> On 30/05/2020 16:29, Mario Frasca wrote:
>> I hope to be back soon with a single commit... 
>
> one doubt.  what's the point of having me squash all in a single commit, 
> when I do not have write access to the repository? 

To provide me or another committer with a polished, final state to
apply, along with a commit message that follows the convention mentioned
at <https://orgmode.org/worg/org-contribute.html> and ideally explains
why the change should be applied.  (The rationale you sent in your first
message could be adapted for this.)  You've been sending a diff,
presumably from the point you branched off of to the tip of your branch.
In that case, you're already presenting each iteration you've sent as
one change; it just lacks a commit message.

Just for clarity: In this case, I think the change proposed so far makes
sense to present as a single commit.  I'm not claiming that in general a
patch series should be reduced to _one_ commit.

> if we were on github, I would be working on a pull request, which
> would have a description and a title, and contain several commits. 

If I were reviewing a pull request from you, I would still request that
you not pile fixup commits on top and that you instead rewrite/polish
the series to address feedback.  People have strong opinions in both
directions on this, and I hope this thread doesn't derail into a
discussion of those.  My point is just that this workflow is not unique
to mailing list patch submission.

Re: `with` as a list.

[Mario Frasca]

On 31/05/2020 19:19, Kyle Meyer wrote:
> You've been sending a diff,
> presumably from the point you branched off of to the tip of your branch.
> In that case, you're already presenting each iteration you've sent as
> one change; it just lacks a commit message.

right, that's indeed what I did, and this is also what I thought, so no 
need to rebase, squash or whatever, as long as I make sure that the diff 
I'm sending you is about this single issue, and let's agree on the 
commit message, because after all I'm adding a function to a software I 
don't really know.

I hope to send an updated patch soon, that will also include the docs.

I have no strong opinion on workflows, just trying to understand the one 
used here.

btw: if I had write permissions to the repositories, I would be adding 
test cases, and reviewing the docstrings, some of which I find 
misleading.  your remark on setf/setq could also be addressed in the 
code.  and some of the code ought to be refactored, as to allow for unit 
tests.

ciao, MF

Re: `with` as a list.

[Kyle Meyer]

Mario Frasca writes:

> On 31/05/2020 19:19, Kyle Meyer wrote:
>> You've been sending a diff,
>> presumably from the point you branched off of to the tip of your branch.
>> In that case, you're already presenting each iteration you've sent as
>> one change; it just lacks a commit message.
>
> right, that's indeed what I did, and this is also what I thought, so no 
> need to rebase, squash or whatever, as long as I make sure that the diff 
> I'm sending you is about this single issue, and let's agree on the 
> commit message, because after all I'm adding a function to a software I 
> don't really know.

The way to add the commit message to the patch you're sending is to
create a commit in your repository with the commit message and then
convert that commit to a patch with `git format-patch'.  When you send
the patch, the commit message can then be reviewed along with the code
change.

The contributing page that I linked to before
(<https://orgmode.org/worg/org-contribute.html>) has some information on
the expected commit message format and on using format-patch.

Re: `with` as a list.

[Mario Frasca]

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

see attachment

[-- Attachment #2: 0001-org-plot.el-implementing-new-feature-with-as-list.patch --]
[-- Type: text/x-patch, Size: 8331 bytes --]

From 2a30f377281955f57723ef46ff56613373cf721d Mon Sep 17 00:00:00 2001
From: mfrasca <mario@anche.no>
Date: Tue, 2 Jun 2020 16:10:11 -0500
Subject: [PATCH] org-plot.el: implementing new feature "with-as-list"

Allows user specify as many `with' values as the columns in `deps'.

User can indicate the order in which the `deps' columns are to be
plotted, this is reflected in the order of the key (legend) elements,
and the z-order of the plotted elements.

User can specify as many different `with`, one for each `deps' column,
in the same order as `deps'.  If you leave the `with` away, you get
"lines" for all columns.  If you specify only one `with` value, that
value is used for all columns.  If you specify more `deps` than
`with`, the ones not specified will get "lines".  If you specify more
`with` than `deps`, they are ignored.

An example:

 #+PLOT: ind:1 deps:(3 6 4 7) with:(points lines points lines)

A more complicated example:

 #+PLOT: set:"style line 2 lw 2 lc rgb 'forest-green'"
 #+PLOT: set:"style line 3 lw 2 lc rgb 'red'"
 #+PLOT: set:"style line 4 lw 1 lc rgb 'red'"
 #+PLOT: ind:1 deps:(3 6 4 7 10)
 #+PLOT: with:("points pt 3 lc rgb 'blue'" "points smooth csplines ls 2" "points pt 19 lc rgb 'blue'" "points smooth csplines ls 3" "points smooth csplines ls 4")

it does not work with histograms.
---
 doc/org-manual.org            |  7 ++--
 lisp/org-plot.el              | 66 +++++++++++++++++++++--------------
 testing/lisp/test-org-plot.el | 64 +++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 29 deletions(-)
 create mode 100644 testing/lisp/test-org-plot.el

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 92252179b..40cb3c2f2 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -2845,9 +2845,10 @@ For more information and examples see the [[https://orgmode.org/worg/org-tutoria
 
 - =with= ::
 
-  Specify a =with= option to be inserted for every column being
-  plotted, e.g., =lines=, =points=, =boxes=, =impulses=.  Defaults to
-  =lines=.
+  Specify a =with= option to be inserted for the columns being
+  plotted, e.g., =lines=, =points=, =boxes=, =impulses=.  You can specify
+  a single value, to be applied to all columns, or a list of different
+  values, one for each column in the =deps= property.  Defaults to =lines=.
 
 - =file= ::
 
diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index a23195d2a..073075037 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -179,6 +179,24 @@ and dependent variables."
 	  (setf back-edge "") (setf front-edge ""))))
     row-vals))
 
+(defun org-plot/zip-deps-with (num-cols ind deps with)
+  "Describe each column to be plotted as (col . with).
+Loops over DEPS and WITH in order to cons their elements.
+If the DEPS list of columns is not given, use all columns from 1
+to NUM-COLS, excluding IND.
+If WITH is given as a string, use the given value for all columns.
+If WITH is given as a list, and it's shorter than DEPS, expand it
+with the global default value."
+  (unless deps
+    (setq deps (remove ind (number-sequence 1 num-cols))))
+  (setq with
+	(if (listp with)
+	    (append with
+		    (make-list (max 0 (- (length deps) (length with)))
+			       "lines"))
+	  (make-list (length deps) with)))
+  (cl-mapcar #'cons deps with))
+
 (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
   "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
 NUM-COLS controls the number of columns plotted in a 2-d plot.
@@ -239,32 +257,27 @@ manner suitable for prepending to a user-specified script."
 			   (or timefmt	; timefmt passed to gnuplot
 			       "%Y-%m-%d-%H:%M:%S") "\"")))
     (unless preface
-      (pcase type			; plot command
-	(`2d (dotimes (col num-cols)
-	       (unless (and (eq type '2d)
-			    (or (and ind (equal (1+ col) ind))
-				(and deps (not (member (1+ col) deps)))))
-		 (setf plot-lines
-		       (cons
-			(format plot-str data-file
-				(or (and ind (> ind 0)
-					 (not text-ind)
-					 (format "%d:" ind)) "")
-				(1+ col)
-				(if text-ind (format ":xticlabel(%d)" ind) "")
-				with
-				(or (nth col col-labels)
-				    (format "%d" (1+ col))))
-			plot-lines)))))
-	(`3d
-	 (setq plot-lines (list (format "'%s' matrix with %s title ''"
-					data-file with))))
-	(`grid
-	 (setq plot-lines (list (format "'%s' with %s title ''"
-					data-file with)))))
+      (setq plot-lines
+	    (pcase type			; plot command
+	      (`2d (cl-loop
+		    for (col . with)
+		    in (org-plot/zip-deps-with num-cols ind deps with)
+		    collect (format plot-str data-file
+				    (or (and ind (> ind 0)
+					     (not text-ind)
+					     (format "%d:" ind)) "")
+				    col
+				    (if text-ind (format ":xticlabel(%d)" ind) "")
+				    with
+				    (or (nth (1- col) col-labels)
+					(format "%d" col)))))
+	      (`3d (list (format "'%s' matrix with %s title ''"
+				 data-file with)))
+	      (`grid (list (format "'%s' with %s title ''"
+				   data-file with)))))
       (funcall ats
 	       (concat plot-cmd " " (mapconcat #'identity
-					       (reverse plot-lines)
+					       plot-lines
 					       ",\\\n    "))))
     script))
 
@@ -310,7 +323,8 @@ line directly before or after the table."
 				table data-file params)))
 		 (when y-labels (plist-put params :ylabels y-labels)))))
       ;; Check for timestamp ind column.
-      (let ((ind (1- (plist-get params :ind))))
+      (let ((ind (1- (plist-get params :ind)))
+	    (with (plist-get params :with)))
 	(when (and (>= ind 0) (eq '2d (plist-get params :plot-type)))
 	  (if (= (length
 		  (delq 0 (mapcar
@@ -320,7 +334,7 @@ line directly before or after the table."
 		 0)
 	      (plist-put params :timeind t)
 	    ;; Check for text ind column.
-	    (if (or (string= (plist-get params :with) "hist")
+	    (if (or (equal with "hist")
 		    (> (length
 			(delq 0 (mapcar
 				 (lambda (el)
diff --git a/testing/lisp/test-org-plot.el b/testing/lisp/test-org-plot.el
new file mode 100644
index 000000000..2bf153400
--- /dev/null
+++ b/testing/lisp/test-org-plot.el
@@ -0,0 +1,64 @@
+;;; test-org-plot.el --- Tests for Org Plot library    -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2020  Mario Frasca
+
+;; Author: Mario Frasca <mario at anche dot no>
+
+;; This file is not part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'org-test)
+(require 'org-plot)
+
+\f
+;; General auxiliaries
+
+(ert-deftest test-org-plot/zip-deps-with ()
+  "Test `org-plot/zip-deps-with' specifications."
+  ;; no deps, no with. defaults to all except ind, and "lines"
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil nil)
+	  '((2 . "lines") (3 . "lines"))))
+  ;; no deps, single with. defaults to all except ind, and repeated with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil "hist")
+	  '((2 . "hist") (3 . "hist"))))
+  ;; no deps, explicit with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil '("points" "hist"))
+	  '((2 . "points") (3 . "hist"))))
+  ;; explicit with, same length as deps
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(2 4) '("points" "hist"))
+	  '((2 . "points") (4 . "hist"))))
+  ;; same as above, but different order
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2) '("points" "hist"))
+	  '((4 . "points") (2 . "hist"))))
+  ;; if with exceeds deps, trailing elements are discarded
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2) '("points" "hist" "lines"))
+	  '((4 . "points") (2 . "hist"))))
+  ;; fills in with "lines"
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2 3) '("points"))
+	  '((4 . "points") (2 . "lines") (3 . "lines")))))
+
+
+\f
+(provide 'test-org-plot)
+;;; test-org-plot.el end here
-- 
2.20.1

Re: `with` as a list.

[Bastien]

Mario Frasca <mario@anche.no> writes:

> see attachment

Thanks for the effort -- the commit message is not correctly formatted
though.  See https://orgmode.org/worg/org-contribute.html#commit-messages
and perhaps also read previous commit messages.

Also you need to sign the FSF copyright assignment if you want to make
big changes like this one.

See https://orgmode.org/request-assign-future.txt

Thanks,

-- 
 Bastien

Re: `with` as a list.

[Mario Frasca]

I'm very so much sorry Bastien, but I do not know what you mean, by "not 
correctly formatted".  if it were a bug report, you do agree it would be 
a useless one?  like "it doesn't work".

I'm sure you do understand what I mean, and I guess you know what would 
be the correctly formatted version.  can you show me?  next time I'll do 
better, but as of now, I obviously do not understand what you want, and 
why.  I was busy amending my other patches, and preparing a few more, 
but let's save time on both sides, and I'll give up for the time being.

ciao,

Mario


On 03/06/2020 10:13, Bastien wrote:
> Mario Frasca <mario@anche.no> writes:
>
>> see attachment
> Thanks for the effort -- the commit message is not correctly formatted
> though.  See https://orgmode.org/worg/org-contribute.html#commit-messages
> and perhaps also read previous commit messages.
>
> Also you need to sign the FSF copyright assignment if you want to make
> big changes like this one.
>
> See https://orgmode.org/request-assign-future.txt
>
> Thanks,
>

Re: `with` as a list.

[Bastien]

Mario Frasca <mario@anche.no> writes:

> I'm sure you do understand what I mean, and I guess you know what
> would be the correctly formatted version.  can you show me?  

Yes.  You mentioned earlier that you missed this section:
 https://orgmode.org/worg/org-contribute.html#commit-messages

Let me quote the text from this section:

A commit message should be constructed in the following way:

    Line 1 of the commit message should always be a short description
    of the overall change. Line 1 does not get a dot at the end and
    does not start with a star. Generally, it starts with the filename
    that has been changed, followed by a colon.

    Line 2 is an empty line.

    In line 3, the ChangeLog entry should start. A ChangeLog entry
    looks like this:

      * org-timer.el (org-timer-cancel-timer, org-timer-stop): Enhance
      message.
      (org-timer-set-timer): Use the number of minutes in the Effort
      property as the default timer value. Three prefix arguments will
      ignore the Effort value property.

The few lines above is what we called the "changelog".  It should be
the first part of the commit message -- after which you can add more
free-form context and explanations, if needed.

When you are on a function/variable that you updated in the modified
version of your code, you can create a changelog entry by hitting the
`C-x 4 a' keybinding.

This will open a new buffer, quote the function or variable, and let
you write a short sentence (starting with an uppercase letter and
ending with the ".", as a regular sentence.)

See these examples:

https://code.orgmode.org/bzg/org-mode/commit/b908367b03
https://code.orgmode.org/bzg/org-mode/commit/65fdf2be16
https://code.orgmode.org/bzg/org-mode/commit/f5997d5956

> next time I'll do better, but as of now, I obviously do not
> understand what you want, and why. 

I hope it is a bit more clear now.

> I was busy amending my other patches, and preparing a few more, but
> let's save time on both sides, and I'll give up for the time being.

You did the hardest part, which is to fix things, I hope you will be 
able to update the formatting of the commit messages.

As for the "why", this is because Org follows the same conventions
than Emacs when it comes to commit messages (partly because Org is
part of Emacs.)

Thanks in advance for when you'll feel motivated again!

-- 
 Bastien

Re: `with` as a list.

[Mario Frasca]

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

fixing things is not as difficult as following stiff rules. anyhow, 
again an attempt, with the terrible feeling I'm wasting my and your 
time.  (looks like I found a printer, for signing the paperwork.)


[-- Attachment #2: 0001-org-plot.el-implementing-new-feature-with-as-list.patch --]
[-- Type: text/x-patch, Size: 8755 bytes --]

From 3375ee101054dc087b40da119941e0433b63fea5 Mon Sep 17 00:00:00 2001
From: mfrasca <mario@anche.no>
Date: Tue, 2 Jun 2020 16:10:11 -0500
Subject: [PATCH] org-plot.el: implementing new feature "with-as-list"

* testing/lisp/test-org-plot.el (test-org-plot/zip-deps-with): new
file, testing the added functionality.
lisp/org-plot.el (org-plot/zip-deps-with): new internal function, used
in modified `org-plot/gnuplot'.
(org-plot/gnuplot): implemented the "with-as-list" feature.
doc/org-manual.org: describing the new feature.

This patch allows user specify as many `with' values as the columns in
`deps'.

User can indicate the order in which the `deps' columns are to be
plotted.  This is reflected in the order of the key (legend) elements,
and the z-order of the plotted elements.

User can specify as many different `with` as the columns to be
plotted, that is one for each `deps' column, in the same order as
`deps'.  If you leave the `with` away, you get "lines" for all columns
(this is hard-coded).  If you specify only one `with` value, that
value is used for all columns.  If you specify more `deps` than
`with`, the ones not specified will get "lines" (again hard-coded).
If you specify more `with` than `deps`, the exceeding elements are
ignored.

An example:

 #+PLOT: ind:1 deps:(3 6 4 7) with:(points lines points lines)

A more complicated example:

 #+PLOT: set:"style line 2 lw 2 lc rgb 'forest-green'"
 #+PLOT: set:"style line 3 lw 2 lc rgb 'red'"
 #+PLOT: set:"style line 4 lw 1 lc rgb 'red'"
 #+PLOT: ind:1 deps:(3 6 4 7 10)
 #+PLOT: with:("points pt 3 lc rgb 'blue'" "points smooth csplines ls 2" "points pt 19 lc rgb 'blue'" "points smooth csplines ls 3" "points smooth csplines ls 4")

It does not work with histograms.
---
 doc/org-manual.org            |  7 ++--
 lisp/org-plot.el              | 66 +++++++++++++++++++++--------------
 testing/lisp/test-org-plot.el | 64 +++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 29 deletions(-)
 create mode 100644 testing/lisp/test-org-plot.el

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 92252179b..40cb3c2f2 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -2845,9 +2845,10 @@ For more information and examples see the [[https://orgmode.org/worg/org-tutoria
 
 - =with= ::
 
-  Specify a =with= option to be inserted for every column being
-  plotted, e.g., =lines=, =points=, =boxes=, =impulses=.  Defaults to
-  =lines=.
+  Specify a =with= option to be inserted for the columns being
+  plotted, e.g., =lines=, =points=, =boxes=, =impulses=.  You can specify
+  a single value, to be applied to all columns, or a list of different
+  values, one for each column in the =deps= property.  Defaults to =lines=.
 
 - =file= ::
 
diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index a23195d2a..073075037 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -179,6 +179,24 @@ and dependent variables."
 	  (setf back-edge "") (setf front-edge ""))))
     row-vals))
 
+(defun org-plot/zip-deps-with (num-cols ind deps with)
+  "Describe each column to be plotted as (col . with).
+Loops over DEPS and WITH in order to cons their elements.
+If the DEPS list of columns is not given, use all columns from 1
+to NUM-COLS, excluding IND.
+If WITH is given as a string, use the given value for all columns.
+If WITH is given as a list, and it's shorter than DEPS, expand it
+with the global default value."
+  (unless deps
+    (setq deps (remove ind (number-sequence 1 num-cols))))
+  (setq with
+	(if (listp with)
+	    (append with
+		    (make-list (max 0 (- (length deps) (length with)))
+			       "lines"))
+	  (make-list (length deps) with)))
+  (cl-mapcar #'cons deps with))
+
 (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
   "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
 NUM-COLS controls the number of columns plotted in a 2-d plot.
@@ -239,32 +257,27 @@ manner suitable for prepending to a user-specified script."
 			   (or timefmt	; timefmt passed to gnuplot
 			       "%Y-%m-%d-%H:%M:%S") "\"")))
     (unless preface
-      (pcase type			; plot command
-	(`2d (dotimes (col num-cols)
-	       (unless (and (eq type '2d)
-			    (or (and ind (equal (1+ col) ind))
-				(and deps (not (member (1+ col) deps)))))
-		 (setf plot-lines
-		       (cons
-			(format plot-str data-file
-				(or (and ind (> ind 0)
-					 (not text-ind)
-					 (format "%d:" ind)) "")
-				(1+ col)
-				(if text-ind (format ":xticlabel(%d)" ind) "")
-				with
-				(or (nth col col-labels)
-				    (format "%d" (1+ col))))
-			plot-lines)))))
-	(`3d
-	 (setq plot-lines (list (format "'%s' matrix with %s title ''"
-					data-file with))))
-	(`grid
-	 (setq plot-lines (list (format "'%s' with %s title ''"
-					data-file with)))))
+      (setq plot-lines
+	    (pcase type			; plot command
+	      (`2d (cl-loop
+		    for (col . with)
+		    in (org-plot/zip-deps-with num-cols ind deps with)
+		    collect (format plot-str data-file
+				    (or (and ind (> ind 0)
+					     (not text-ind)
+					     (format "%d:" ind)) "")
+				    col
+				    (if text-ind (format ":xticlabel(%d)" ind) "")
+				    with
+				    (or (nth (1- col) col-labels)
+					(format "%d" col)))))
+	      (`3d (list (format "'%s' matrix with %s title ''"
+				 data-file with)))
+	      (`grid (list (format "'%s' with %s title ''"
+				   data-file with)))))
       (funcall ats
 	       (concat plot-cmd " " (mapconcat #'identity
-					       (reverse plot-lines)
+					       plot-lines
 					       ",\\\n    "))))
     script))
 
@@ -310,7 +323,8 @@ line directly before or after the table."
 				table data-file params)))
 		 (when y-labels (plist-put params :ylabels y-labels)))))
       ;; Check for timestamp ind column.
-      (let ((ind (1- (plist-get params :ind))))
+      (let ((ind (1- (plist-get params :ind)))
+	    (with (plist-get params :with)))
 	(when (and (>= ind 0) (eq '2d (plist-get params :plot-type)))
 	  (if (= (length
 		  (delq 0 (mapcar
@@ -320,7 +334,7 @@ line directly before or after the table."
 		 0)
 	      (plist-put params :timeind t)
 	    ;; Check for text ind column.
-	    (if (or (string= (plist-get params :with) "hist")
+	    (if (or (equal with "hist")
 		    (> (length
 			(delq 0 (mapcar
 				 (lambda (el)
diff --git a/testing/lisp/test-org-plot.el b/testing/lisp/test-org-plot.el
new file mode 100644
index 000000000..2bf153400
--- /dev/null
+++ b/testing/lisp/test-org-plot.el
@@ -0,0 +1,64 @@
+;;; test-org-plot.el --- Tests for Org Plot library    -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2020  Mario Frasca
+
+;; Author: Mario Frasca <mario at anche dot no>
+
+;; This file is not part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'org-test)
+(require 'org-plot)
+
+\f
+;; General auxiliaries
+
+(ert-deftest test-org-plot/zip-deps-with ()
+  "Test `org-plot/zip-deps-with' specifications."
+  ;; no deps, no with. defaults to all except ind, and "lines"
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil nil)
+	  '((2 . "lines") (3 . "lines"))))
+  ;; no deps, single with. defaults to all except ind, and repeated with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil "hist")
+	  '((2 . "hist") (3 . "hist"))))
+  ;; no deps, explicit with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil '("points" "hist"))
+	  '((2 . "points") (3 . "hist"))))
+  ;; explicit with, same length as deps
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(2 4) '("points" "hist"))
+	  '((2 . "points") (4 . "hist"))))
+  ;; same as above, but different order
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2) '("points" "hist"))
+	  '((4 . "points") (2 . "hist"))))
+  ;; if with exceeds deps, trailing elements are discarded
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2) '("points" "hist" "lines"))
+	  '((4 . "points") (2 . "hist"))))
+  ;; fills in with "lines"
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2 3) '("points"))
+	  '((4 . "points") (2 . "lines") (3 . "lines")))))
+
+
+\f
+(provide 'test-org-plot)
+;;; test-org-plot.el end here
-- 
2.20.1

Re: `with` as a list.

[Mario Frasca]

On 03/06/2020 10:29, Bastien wrote:
> The few lines above is what we called the "changelog".  It should be
> the first part of the commit message -- after which you can add more
> free-form context and explanations, if needed.

this "Changelog", and I'm sure I am the stiffy one, isn't clear from the 
description.  browsing through the other contributions, I recognize this 
structure:

the "Changelog" is the second block, (preferably) without any empty 
lines.  the next empty line will end the "Changelog" and start the less 
formal comments. (unsure about this).

changes are grouped by file, each changed file is introduced by a line 
starting with an asterisk '*', and the complete path of the file.  then 
follows the function affected, in single parentheses, a colon ':', and 
the description of the change.  if the change is relative to 
documentation, the parentheses include title of the affected paragraph.  
finally the description, formatted as per your html page.

if the next change is in the same file, you skip the leading '*' and 
file name and the line should start with the parenthesized affected object.

----------------------------

for example (two files, two different changes --- code and doc):

* lisp/org.el (org-read-date-analyze): Add support for HHhMM time
input, in similar way as for am/pm times.
* doc/org-manual.org (The date/time prompt): Add example to illustrate
the feature.

----------------------------

other example (two affected files, single description --- code):

* testing/lisp/test-ob-tangle.el (ob-tangle/jump-to-org):
* testing/lisp/test-org-attach.el (test-org-attach/dir): Rig
org-file-apps so that temporary files are visited inside Emacs.

----------------------------

(one file, two different changes --- doc):

* doc/org-manual.org (Capturing column view): Replace stale binding
with mention of org-dynamic-block-insert-dblock, and refer to
org-columns-insert-dblock rather than its obsolete variant.
(The clock table): Prune references to stale binding, rewrite
org-dynamic-block-insert-dblock key sequence in a clearer manner, and
add a dedicated entry for org-clock-report.

----------------------------

slightly sloppy example (two files, single description, not clear what's 
affected):

* lisp/org-clock.el:
* lisp/org-colview.el: Autoload call to org-dynamic-block-define.

----------------------------

etc/ORG-NEWS is an exception, and should be mentioned as first.
are empty lines acceptable?  or will your parser fail to understand?

* etc/ORG-NEWS: Announce the change.

* lisp/org-keys.el (org-mode-map): Rebind C-j to a command emulating
`electric-newline-and-maybe-indent'.

* lisp/org.el (org-cdlatex-environment-indent): Stop using the now
obsolete function.
(org--newline): New helper function.
(org-return): Use it to transparently handle `electric-indent-mode'.
(org-return-and-maybe-indent): New command to emulate
`electric-newline-and-maybe-indent' while taking care of Org special
cases (tables, links, timestamps).

----------------------------

Re: `with` as a list.

[Mario Frasca]

On 03/06/2020 10:13, Bastien wrote:
> Also you need to sign the FSF copyright assignment if you want to make
> big changes like this one.
yes, I received the ASSIGNMENT –GNU EMACS pdf last night, I'm now seeing 
how I print & sign it.  it might take some time.

Re: `with` as a list.

[Bastien]

Mario Frasca <mario@anche.no> writes:

> yes, I received the ASSIGNMENT –GNU EMACS pdf last night, I'm now
> seeing how I print & sign it.  it might take some time.

Sure - probably a few weeks.  Let us know when this is done so that 
we can go ahead with applying your patches.

-- 
 Bastien

Re: `with` as a list.

[Mario Frasca]

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

what's new …

edited the documentation string to `org-plot/zip-deps-with',

removed some redundancies there,

removed my ›zip‹ function, in favour of ›cl-mapcar 'cons‹,

simplified the double test `stringp', `string=' with `equal'.

simplified the unit tests correspondingly.

make test: no unexpected failing tests.


[-- Attachment #2: org-plot.diff --]
[-- Type: text/x-patch, Size: 5251 bytes --]

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index a23195d2a..c44cca991 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -156,7 +156,8 @@ and dependent variables."
 			  table)))
     ;; write table to gnuplot grid datafile format
     (with-temp-file data-file
-      (let ((num-rows (length table)) (num-cols (length (nth 0 table)))
+      (let ((num-rows (length table))
+	    (num-cols (length (nth 0 table)))
 	    (gnuplot-row (lambda (col row value)
 			   (setf col (+ 1 col)) (setf row (+ 1 row))
 			   (format "%f  %f  %f\n%f  %f  %f\n"
@@ -179,6 +180,22 @@ and dependent variables."
 	  (setf back-edge "") (setf front-edge ""))))
     row-vals))
 
+(defun org-plot/zip-deps-with (num-cols ind deps with)
+  "Describe each column to be plotted as (col . with).
+Loops over DEPS and WITH in order to cons their elements.
+If the DEPS list of columns is not given, use all columns from 1
+to NUM-COLS, excluding IND.
+If WITH is given as a string, use the given value for all
+columns.
+If WITH is given as a list, and it's shorter than DEPS, expand it
+with the global default value."
+  (unless deps
+    (setq deps (remove ind (number-sequence 1 num-cols))))
+  (unless (listp with)
+    (setq with (make-list (length deps) with)))
+  (setq with (append with (make-list (- (length deps) (length with)) "lines")))
+  (cl-mapcar 'cons deps with))
+
 (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
   "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
 NUM-COLS controls the number of columns plotted in a 2-d plot.
@@ -240,22 +257,24 @@ manner suitable for prepending to a user-specified script."
 			       "%Y-%m-%d-%H:%M:%S") "\"")))
     (unless preface
       (pcase type			; plot command
-	(`2d (dotimes (col num-cols)
-	       (unless (and (eq type '2d)
-			    (or (and ind (equal (1+ col) ind))
-				(and deps (not (member (1+ col) deps)))))
-		 (setf plot-lines
-		       (cons
-			(format plot-str data-file
-				(or (and ind (> ind 0)
-					 (not text-ind)
-					 (format "%d:" ind)) "")
-				(1+ col)
-				(if text-ind (format ":xticlabel(%d)" ind) "")
-				with
-				(or (nth col col-labels)
-				    (format "%d" (1+ col))))
-			plot-lines)))))
+	(`2d (dolist
+		 (col-with
+		  (org-plot/zip-deps-with num-cols ind deps with))
+	       (setf plot-lines
+		     (cons
+		      (format plot-str data-file
+			      (or (and ind (> ind 0)
+				       (not text-ind)
+				       (format "%d:" ind)) "")
+			      (car col-with)
+			      (if text-ind (format ":xticlabel(%d)" ind) "")
+			      (cdr col-with)
+			      (apply (lambda (x)
+				       (if (= 0 (length x))
+					   (format "%d" (car col-with))
+					 x))
+				     (list (nth (1- (car col-with)) col-labels))))
+		      plot-lines))))
 	(`3d
 	 (setq plot-lines (list (format "'%s' matrix with %s title ''"
 					data-file with))))
@@ -310,7 +329,8 @@ line directly before or after the table."
 				table data-file params)))
 		 (when y-labels (plist-put params :ylabels y-labels)))))
       ;; Check for timestamp ind column.
-      (let ((ind (1- (plist-get params :ind))))
+      (let ((ind (1- (plist-get params :ind)))
+	    (with (plist-get params :with)))
 	(when (and (>= ind 0) (eq '2d (plist-get params :plot-type)))
 	  (if (= (length
 		  (delq 0 (mapcar
@@ -320,7 +340,7 @@ line directly before or after the table."
 		 0)
 	      (plist-put params :timeind t)
 	    ;; Check for text ind column.
-	    (if (or (string= (plist-get params :with) "hist")
+	    (if (or (equal with "hist")
 		    (> (length
 			(delq 0 (mapcar
 				 (lambda (el)
diff --git a/testing/lisp/test-org-plot.el b/testing/lisp/test-org-plot.el
new file mode 100644
index 000000000..2bbd09b5f
--- /dev/null
+++ b/testing/lisp/test-org-plot.el
@@ -0,0 +1,50 @@
+;;; test-org-plot.el --- Tests for org-plot.el
+
+;; Copyright (C) 2020  Mario Frasca
+
+;; Author: Mario Frasca <mario at anche dot no>
+
+;; Released under the GNU General Public License version 3
+;; see: http://www.gnu.org/licenses/gpl-3.0.html
+
+;;;; Comments
+
+
+\f
+;;; Code:
+
+(require 'org-test)
+(require 'org-plot)
+
+\f
+;; General auxiliaries
+
+(ert-deftest test-org-plot/zip-deps-with ()
+  "Test `org-plot/zip-deps-with' specifications."
+  ;; no deps, no with. defaults to all except ind, and "lines"
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil nil)
+	  '((2 . "lines") (3 . "lines"))))
+  ;; no deps, single with. defaults to all except ind, and repeated with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil "hist")
+	  '((2 . "hist") (3 . "hist"))))
+  ;; no deps, explicit with
+  (should
+   (equal (org-plot/zip-deps-with 3 1 nil '("points" "hist"))
+	  '((2 . "points") (3 . "hist"))))
+  ;; explicit with, same length as deps
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(2 4) '("points" "hist"))
+	  '((2 . "points") (4 . "hist"))))
+  ;; same as above, but different order
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2) '("points" "hist"))
+	  '((4 . "points") (2 . "hist"))))
+  ;; fills in with "lines"
+  (should
+   (equal (org-plot/zip-deps-with 5 1 '(4 2 3) '("points"))
+	  '((4 . "points") (2 . "lines") (3 . "lines")))))
+\f
+(provide 'test-org-plot)
+;;; test-org-plot.el end here

Re: `with` as a list.

[Kyle Meyer]

Mario Frasca writes:

> diff --git a/lisp/org-plot.el b/lisp/org-plot.el
> index a23195d2a..c44cca991 100644
> --- a/lisp/org-plot.el
> +++ b/lisp/org-plot.el
> @@ -156,7 +156,8 @@ and dependent variables."
>  			  table)))
>      ;; write table to gnuplot grid datafile format
>      (with-temp-file data-file
> -      (let ((num-rows (length table)) (num-cols (length (nth 0 table)))
> +      (let ((num-rows (length table))
> +	    (num-cols (length (nth 0 table)))

I don't disagree with the style preference here, but it's easier on
reviewers if the patch doesn't contain unrelated changes.

>  	    (gnuplot-row (lambda (col row value)
>  			   (setf col (+ 1 col)) (setf row (+ 1 row))
>  			   (format "%f  %f  %f\n%f  %f  %f\n"
> @@ -179,6 +180,22 @@ and dependent variables."
>  	  (setf back-edge "") (setf front-edge ""))))
>      row-vals))
>  
> +(defun org-plot/zip-deps-with (num-cols ind deps with)
> +  "Describe each column to be plotted as (col . with).
> +Loops over DEPS and WITH in order to cons their elements.
> +If the DEPS list of columns is not given, use all columns from 1
> +to NUM-COLS, excluding IND.
> +If WITH is given as a string, use the given value for all
> +columns.
> +If WITH is given as a list, and it's shorter than DEPS, expand it
> +with the global default value."

Thanks for updating the docstring.

> +  (unless deps
> +    (setq deps (remove ind (number-sequence 1 num-cols))))
> +  (unless (listp with)
> +    (setq with (make-list (length deps) with)))
> +  (setq with (append with (make-list (- (length deps) (length with)) "lines")))

The caller can crash this with something like

    make-list(-1 "lines")
    (append with (make-list (- (length deps) (length with)) "lines"))
    (setq with (append with (make-list (- (length deps) (length with)) "lines")))
    org-plot/zip-deps-with(3 1 (2 3) (lines points lines))
    [...]

if they accidentally give more `with` values than `deps`.

Also, if the `(unless (listp with) ...` condition is entered, I think
the second make-list call is unnecessary.

So, combining those two points, perhaps something like this:

    (setq with (if (listp with)
                   (append with
                           (make-list (max 0 (- (length deps) (length with)))
                                      "lines"))
                 (make-list (length deps) with)))

> +  (cl-mapcar 'cons deps with))

Nit-pick: s/'/#'/

The latter is short for `function' and, unlike the former, lets the
byte-compiler know that cons is a function (enabling a warning if, say,
you accidentally typed `conss`).

>  (defun org-plot/gnuplot-script (data-file num-cols params &optional preface)
>    "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS.
>  NUM-COLS controls the number of columns plotted in a 2-d plot.
> @@ -240,22 +257,24 @@ manner suitable for prepending to a user-specified script."
>  			       "%Y-%m-%d-%H:%M:%S") "\"")))
>      (unless preface
>        (pcase type			; plot command
> -	(`2d (dotimes (col num-cols)
> -	       (unless (and (eq type '2d)
> -			    (or (and ind (equal (1+ col) ind))
> -				(and deps (not (member (1+ col) deps)))))
> -		 (setf plot-lines
> -		       (cons
> -			(format plot-str data-file
> -				(or (and ind (> ind 0)
> -					 (not text-ind)
> -					 (format "%d:" ind)) "")
> -				(1+ col)
> -				(if text-ind (format ":xticlabel(%d)" ind) "")
> -				with
> -				(or (nth col col-labels)
> -				    (format "%d" (1+ col))))
> -			plot-lines)))))
> +	(`2d (dolist
> +		 (col-with
> +		  (org-plot/zip-deps-with num-cols ind deps with))
> +	       (setf plot-lines
> +		     (cons
> +		      (format plot-str data-file
> +			      (or (and ind (> ind 0)
> +				       (not text-ind)
> +				       (format "%d:" ind)) "")
> +			      (car col-with)
> +			      (if text-ind (format ":xticlabel(%d)" ind) "")
> +			      (cdr col-with)
> +			      (apply (lambda (x)
> +				       (if (= 0 (length x))
> +					   (format "%d" (car col-with))
> +					 x))
> +				     (list (nth (1- (car col-with)) col-labels))))

If I'm reading it correctly, (apply ...) could be simplified to

    (or (nth (1- (car col-with)) col-labels)
        (format "%d" (car col-with)))

> diff --git a/testing/lisp/test-org-plot.el b/testing/lisp/test-org-plot.el
> new file mode 100644
> index 000000000..2bbd09b5f
> --- /dev/null
> +++ b/testing/lisp/test-org-plot.el
> @@ -0,0 +1,50 @@
> +;;; test-org-plot.el --- Tests for org-plot.el
> +
> +;; Copyright (C) 2020  Mario Frasca
> +
> +;; Author: Mario Frasca <mario at anche dot no>
> +
> +;; Released under the GNU General Public License version 3
> +;; see: http://www.gnu.org/licenses/gpl-3.0.html

Could you update this header to match the style used by other tests
(see, e.g., test-org-num.el)?

Tests themselves look good to me.  Thanks for adding them.  I think it'd
also be good to add a test for the with-longer-than-deps case I
mentioned above.

The manual should also be updated to mention this new feature, and it'd
be good to have an ORG-NEWS entry.

Thanks for working on this.

Re: `with` as a list.

[Mario Frasca]

On 30/05/2020 15:25, Kyle Meyer wrote:
> Could you update this header to match the style used by other tests
> (see, e.g., test-org-num.el)?

apparently, I managed to pick precisely the wrong example!

;;; test-org-clock.el --- Tests for org-clock.el

Re: `with` as a list.

[Kyle Meyer]

Mario Frasca writes:

> On 30/05/2020 15:25, Kyle Meyer wrote:
>> Could you update this header to match the style used by other tests
>> (see, e.g., test-org-num.el)?
>
> apparently, I managed to pick precisely the wrong example!
>
> ;;; test-org-clock.el --- Tests for org-clock.el

Ha, I poked around in a few and couldn't find the one that looked like
yours, even though I figured it was based off another one.  I think
either way is fine, though it probably makes sense to go with the fuller
header that seems to be more common.