[BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

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


Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

Consider the following table, and try to execute the PLOT line

    #+PLOT: ind:1 deps:(3) type:2d with:lp
    | Sede      | Max cites | H-index |
    |-----------+-----------+---------|
    | Chile     |    257.72 |   21.39 |
    | Leeds     |    165.77 |   19.68 |
    | Sao Paolo |     71.00 |   11.50 |
    | Stockholm |    134.19 |   14.33 |
    | Morelia   |    257.56 |   17.67 |

and watch it fail (see the error message in the *gnuplot* buffer).
Although the 2d option in org-plot/preset-plot-types has :check-ind-type
set to a non-nil value, org-plot/gnuplot does not check the value of
:check-ind-type assigned in the 2d type given in the user-option.

Unless I misunderstood the code, the line

      ;; Check type of ind column (timestamp? text?)
      (when (plist-get params :check-ind-type)

should be

      ;; Check type of ind column (timestamp? text?)
      (when (plist-get (cdr type) :check-ind-type)

because (1) org-plot/collect-options only adds a select number of
keywords to the plist and :check-ind-type is not a part of the select
members, and (2) org-plot/gnuplot is never called with a non-nil value
for the optional argument PARAMS in tree.

BTW, the earlier check in the function for :data-dump should also fail
because

    (plist-get (assoc 'grid org-plot/preset-plot-types) :data-dump) ;; => nil

but

    (plist-get (cdr (assoc 'grid org-plot/preset-plot-types)) :data-dump) ;; => non-nil

where type ≡ (assoc 'grid org-plot/preset-plot-types) in
org-plot/gnuplot.

[ I cannot reproduce the grid example in worg's org-plot.org file, but
  even with the fix, I cannot reproduce it; more below.  ]

The other code smell I see is that the function checks for the PLOT line
twice.  Once near the beginning of the function, and once just after the
cleaning up of hline.  Is this simply an oversight?

Coming to the grid example, the doc-string of org-plot/preset-plot-types
options says:

    - :data-dump - Function to dump the table to a datafile for ease of
      use.

      Accepts lambda function.  Default lambda body:
      (org-plot/gnuplot-to-data table data-file params)

but in fact, org-plot/gnuplot passes one more argument to the :data-dump
function:

      ;; Dump table to datafile
      (let ((dump-func (plist-get type :data-dump)))
        (if dump-func
	    (funcall dump-func table data-file num-cols params)
	  (org-plot/gnuplot-to-data table data-file params)))

but here's the catch: the :data-dump function in the grid option expects
the order

    (lambda (table data-file params _num-cols)

which breaks things down the line.  What should be the actual order
here?  I looked at the history of those lines briefly using C-x v h but
I don't have the time to look into it properly to decide on the actual
argument order.

For now, I have attached a patch that fixes all the issues.  With the
patch, i can run the example covered in the bug report.  The grid
example given in worg passes but it doesn't look as expected.  I am not
sure what to blame here.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-plot-fix.diff --]
[-- Type: text/x-diff, Size: 1505 bytes --]

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index 283d993..3be1b2f 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -679,8 +679,8 @@ (defun org-plot/gnuplot (&optional params)
 		    tbl))
 	   (num-cols (length (if (eq (nth 0 table) 'hline) (nth 1 table)
 			       (nth 0 table))))
-	   (type (assoc (plist-get params :plot-type)
-			org-plot/preset-plot-types))
+	   (type (cdr (assoc (plist-get params :plot-type)
+			     org-plot/preset-plot-types)))
            gnuplot-script)

       (unless type
@@ -691,17 +691,13 @@ (defun org-plot/gnuplot (&optional params)
 	(setf params
 	      (plist-put params :labels (car table))) ; headers to labels
 	(setf table (delq 'hline (cdr table)))) ; clean non-data from table
-      ;; Collect options.
-      (save-excursion (while (and (equal 0 (forward-line -1))
-				  (looking-at "[[:space:]]*#\\+"))
-			(setf params (org-plot/collect-options params))))
       ;; Dump table to datafile
       (let ((dump-func (plist-get type :data-dump)))
         (if dump-func
-	    (funcall dump-func table data-file num-cols params)
+	    (funcall dump-func table data-file params num-cols)
 	  (org-plot/gnuplot-to-data table data-file params)))
       ;; Check type of ind column (timestamp? text?)
-      (when (plist-get params :check-ind-type)
+      (when (plist-get type :check-ind-type)
 	(let* ((ind (1- (plist-get params :ind)))
 	       (ind-column (mapcar (lambda (row) (nth ind row)) table)))
 	  (cond ((< ind 0) nil) ; ind is implicit

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


Emacs  : GNU Emacs 30.0.50 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo version 1.18.0, Xaw scroll bars)
 of 2024-05-07
Package: Org mode version 9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

> Unless I misunderstood the code, the line
>
>       ;; Check type of ind column (timestamp? text?)
>       (when (plist-get params :check-ind-type)
>
> should be
>
>       ;; Check type of ind column (timestamp? text?)
>       (when (plist-get (cdr type) :check-ind-type)
>
> because (1) org-plot/collect-options only adds a select number of
> keywords to the plist and :check-ind-type is not a part of the select
> members, and (2) org-plot/gnuplot is never called with a non-nil value
> for the optional argument PARAMS in tree.

I do not think that it is right.
AFAIU, the idea is that `org-plot/preset-plot-types' provides some
default options, but the user can overwrite these defaults in the #+PLOT
line. What you propose will disregard the values of

 :set :line :map :title :file :ind :timeind :timefmt :textind
 :deps :labels :xlabels :ylabels :xmin :xmax :ymin :ymax :ticks

if they are customized by user in `org-plot/preset-plot-types'.

I believe that the right way to address the problem will be
`org-combine-plists' on the (1) org-plot/preset-plot-types; (2)
org-plot/gnuplot-default-options; (3) #+PLOT lines in the buffer.

> BTW, the earlier check in the function for :data-dump should also fail
> because
>
>     (plist-get (assoc 'grid org-plot/preset-plot-types) :data-dump) ;; => nil
>
> but
>
>     (plist-get (cdr (assoc 'grid org-plot/preset-plot-types)) :data-dump) ;; => non-nil
>
> where type ≡ (assoc 'grid org-plot/preset-plot-types) in
> org-plot/gnuplot.
>
> [ I cannot reproduce the grid example in worg's org-plot.org file, but
>   even with the fix, I cannot reproduce it; more below.  ]

Your concern is valid here. The code is indeed not right.

> The other code smell I see is that the function checks for the PLOT line
> twice.  Once near the beginning of the function, and once just after the
> cleaning up of hline.  Is this simply an oversight?

It is kinda intentional, but broken.

Historically, users can put #+PLOT lines _after_ the table.
However, after refactoring org-table.el, this is no longer working.
See https://list.orgmode.org/orgmode/87o7a0p9ba.fsf@localhost/

> Coming to the grid example, the doc-string of org-plot/preset-plot-types
> options says:
>
>     - :data-dump - Function to dump the table to a datafile for ease of
>       use.
>
>       Accepts lambda function.  Default lambda body:
>       (org-plot/gnuplot-to-data table data-file params)
>
> but in fact, org-plot/gnuplot passes one more argument to the :data-dump
> function:
>
>       ;; Dump table to datafile
>       (let ((dump-func (plist-get type :data-dump)))
>         (if dump-func
> 	    (funcall dump-func table data-file num-cols params)
> 	  (org-plot/gnuplot-to-data table data-file params)))
>
> but here's the catch: the :data-dump function in the grid option expects
> the order
>
>     (lambda (table data-file params _num-cols)
>
> which breaks things down the line.  What should be the actual order
> here?  I looked at the history of those lines briefly using C-x v h but
> I don't have the time to look into it properly to decide on the actual
> argument order.

The best order is de-facto calling convention in the code:

(funcall dump-func table data-file num-cols params)

The docstring and the default value should be fixed accordingly.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

[வெள்ளி மே 24, 2024] Ihor Radchenko wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> Unless I misunderstood the code, the line
>>
>>       ;; Check type of ind column (timestamp? text?)
>>       (when (plist-get params :check-ind-type)
>>
>> should be
>>
>>       ;; Check type of ind column (timestamp? text?)
>>       (when (plist-get (cdr type) :check-ind-type)
>>
>> because (1) org-plot/collect-options only adds a select number of
>> keywords to the plist and :check-ind-type is not a part of the select
>> members, and (2) org-plot/gnuplot is never called with a non-nil value
>> for the optional argument PARAMS in tree.
>
> I do not think that it is right.
> AFAIU, the idea is that `org-plot/preset-plot-types' provides some
> default options, but the user can overwrite these defaults in the #+PLOT
> line. What you propose will disregard the values of
>
>  :set :line :map :title :file :ind :timeind :timefmt :textind
>  :deps :labels :xlabels :ylabels :xmin :xmax :ymin :ymax :ticks
>
> if they are customized by user in `org-plot/preset-plot-types'.

I don't follow your conclusion since this change will only affect the
value of :check-ind-type leaving the rest of the settings unaffected.

> I believe that the right way to address the problem will be
> `org-combine-plists' on the (1) org-plot/preset-plot-types; (2)
> org-plot/gnuplot-default-options; (3) #+PLOT lines in the buffer.

Looking at the definition of org-plot/add-options-to-plist and the Info
manual, I am not sure if :check-ind-type is supposed to be customised by
the PLOT line.  It seems to be more of an internal setting.  If you
agree to this, I can change the check to

    (when (or (plist-get type :check-ind-type) (plist-get params :check-ind-type))

to heed the value of :check-ind-type in org-plot/gnuplot-default-options
(since PARAMS has the default values included earlier in the defun).

>> [...]
>> The other code smell I see is that the function checks for the PLOT line
>> twice.  Once near the beginning of the function, and once just after the
>> cleaning up of hline.  Is this simply an oversight?
>
> It is kinda intentional, but broken.
>
> Historically, users can put #+PLOT lines _after_ the table.
> However, after refactoring org-table.el, this is no longer working.
> See https://list.orgmode.org/orgmode/87o7a0p9ba.fsf@localhost/

OK, I will leave the check in then.

>> Coming to the grid example, the doc-string of org-plot/preset-plot-types
>> options says:
>>
>>     - :data-dump - Function to dump the table to a datafile for ease of
>>       use.
>>
>>       Accepts lambda function.  Default lambda body:
>>       (org-plot/gnuplot-to-data table data-file params)
>>
>> but in fact, org-plot/gnuplot passes one more argument to the :data-dump
>> function:
>>
>>       ;; Dump table to datafile
>>       (let ((dump-func (plist-get type :data-dump)))
>>         (if dump-func
>> 	    (funcall dump-func table data-file num-cols params)
>> 	  (org-plot/gnuplot-to-data table data-file params)))
>>
>> but here's the catch: the :data-dump function in the grid option expects
>> the order
>>
>>     (lambda (table data-file params _num-cols)
>>
>> which breaks things down the line.  What should be the actual order
>> here?  I looked at the history of those lines briefly using C-x v h but
>> I don't have the time to look into it properly to decide on the actual
>> argument order.
>
> The best order is de-facto calling convention in the code:
>
> (funcall dump-func table data-file num-cols params)
>
> The docstring and the default value should be fixed accordingly.

OK, will do this in a future patch.

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

Ping.  Can you answer my questions, Ihor, so that I may prepare a patch
accordingly?

[வெள்ளி மே 24, 2024] Visuwesh wrote:

> [வெள்ளி மே 24, 2024] Ihor Radchenko wrote:
>
>> Visuwesh <visuweshm@gmail.com> writes:
>>
>>> Unless I misunderstood the code, the line
>>>
>>>       ;; Check type of ind column (timestamp? text?)
>>>       (when (plist-get params :check-ind-type)
>>>
>>> should be
>>>
>>>       ;; Check type of ind column (timestamp? text?)
>>>       (when (plist-get (cdr type) :check-ind-type)
>>>
>>> because (1) org-plot/collect-options only adds a select number of
>>> keywords to the plist and :check-ind-type is not a part of the select
>>> members, and (2) org-plot/gnuplot is never called with a non-nil value
>>> for the optional argument PARAMS in tree.
>>
>> I do not think that it is right.
>> AFAIU, the idea is that `org-plot/preset-plot-types' provides some
>> default options, but the user can overwrite these defaults in the #+PLOT
>> line. What you propose will disregard the values of
>>
>>  :set :line :map :title :file :ind :timeind :timefmt :textind
>>  :deps :labels :xlabels :ylabels :xmin :xmax :ymin :ymax :ticks
>>
>> if they are customized by user in `org-plot/preset-plot-types'.
>
> I don't follow your conclusion since this change will only affect the
> value of :check-ind-type so how would it affect the rest of the
> settings?
>
>> I believe that the right way to address the problem will be
>> `org-combine-plists' on the (1) org-plot/preset-plot-types; (2)
>> org-plot/gnuplot-default-options; (3) #+PLOT lines in the buffer.
>
> Looking at the definition of org-plot/add-options-to-plist and the Info
> manual, I am not sure if :check-ind-type is supposed to be customised by
> the PLOT line.  It seems to be more of an internal setting.  If you
> agree to this, I can change the check to
>
>     (when (or (plist-get type :check-ind-type) (plist-get params :check-ind-type))
>
> to heed the value of :check-ind-type in org-plot/gnuplot-default-options
> (since PARAMS has the default values included earlier in the defun).
>
>>> [...]
>>> The other code smell I see is that the function checks for the PLOT line
>>> twice.  Once near the beginning of the function, and once just after the
>>> cleaning up of hline.  Is this simply an oversight?
>>
>> It is kinda intentional, but broken.
>>
>> Historically, users can put #+PLOT lines _after_ the table.
>> However, after refactoring org-table.el, this is no longer working.
>> See https://list.orgmode.org/orgmode/87o7a0p9ba.fsf@localhost/
>
> OK, I will leave the check in then.
>
>>> Coming to the grid example, the doc-string of org-plot/preset-plot-types
>>> options says:
>>>
>>>     - :data-dump - Function to dump the table to a datafile for ease of
>>>       use.
>>>
>>>       Accepts lambda function.  Default lambda body:
>>>       (org-plot/gnuplot-to-data table data-file params)
>>>
>>> but in fact, org-plot/gnuplot passes one more argument to the :data-dump
>>> function:
>>>
>>>       ;; Dump table to datafile
>>>       (let ((dump-func (plist-get type :data-dump)))
>>>         (if dump-func
>>> 	    (funcall dump-func table data-file num-cols params)
>>> 	  (org-plot/gnuplot-to-data table data-file params)))
>>>
>>> but here's the catch: the :data-dump function in the grid option expects
>>> the order
>>>
>>>     (lambda (table data-file params _num-cols)
>>>
>>> which breaks things down the line.  What should be the actual order
>>> here?  I looked at the history of those lines briefly using C-x v h but
>>> I don't have the time to look into it properly to decide on the actual
>>> argument order.
>>
>> The best order is de-facto calling convention in the code:
>>
>> (funcall dump-func table data-file num-cols params)
>>
>> The docstring and the default value should be fixed accordingly.
>
> OK, will do this in a future patch.

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

>> I do not think that it is right.
>> AFAIU, the idea is that `org-plot/preset-plot-types' provides some
>> default options, but the user can overwrite these defaults in the #+PLOT
>> line. What you propose will disregard the values of
>>
>>  :set :line :map :title :file :ind :timeind :timefmt :textind
>>  :deps :labels :xlabels :ylabels :xmin :xmax :ymin :ymax :ticks
>>
>> if they are customized by user in `org-plot/preset-plot-types'.
>
> I don't follow your conclusion since this change will only affect the
> value of :check-ind-type leaving the rest of the settings unaffected.

My point is that we will eventually need to merge TYPE and PARAMS to
fix another bug - `org-plot/preset-plot-types' options like :set, :line,
etc being ignored. So, instead of patching the way you proposed, we can
merge TYPE and PARAMS into PARAMS, making the existing (plist-get params
:check-ind-type) working.

In other words, org-plot's handling of parameters is very broken
now. There is more than one bug lurking there, and it may be more
productive to fix things together.

You solution will, of course, work, but only for this specific bug you
described; not for other.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

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

[புதன் ஜூன் 12, 2024] Ihor Radchenko wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>>> I do not think that it is right.
>>> AFAIU, the idea is that `org-plot/preset-plot-types' provides some
>>> default options, but the user can overwrite these defaults in the #+PLOT
>>> line. What you propose will disregard the values of
>>>
>>>  :set :line :map :title :file :ind :timeind :timefmt :textind
>>>  :deps :labels :xlabels :ylabels :xmin :xmax :ymin :ymax :ticks
>>>
>>> if they are customized by user in `org-plot/preset-plot-types'.
>>
>> I don't follow your conclusion since this change will only affect the
>> value of :check-ind-type leaving the rest of the settings unaffected.
>
> My point is that we will eventually need to merge TYPE and PARAMS to
> fix another bug - `org-plot/preset-plot-types' options like :set, :line,
> etc being ignored. So, instead of patching the way you proposed, we can
> merge TYPE and PARAMS into PARAMS, making the existing (plist-get params
> :check-ind-type) working.
>
> In other words, org-plot's handling of parameters is very broken
> now. There is more than one bug lurking there, and it may be more
> productive to fix things together.
>
> You solution will, of course, work, but only for this specific bug you
> described; not for other.

OK, thanks for the explanation.  I was unaware of the issue as I didn't
read the code closely enough.  What about the attached patch?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-plot-Respect-parameters-given-in-org-plot-preset.patch --]
[-- Type: text/x-diff, Size: 2527 bytes --]

From 2a153a5bc015b0e970ecde39fd2edbb515261349 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Thu, 13 Jun 2024 12:33:49 +0530
Subject: [PATCH] org-plot: Respect parameters given in
 `org-plot/preset-plot-types'

* lisp/org-plot.el (org-plot/preset-plot-types): Fix docstring and
correct the lambda argument order for the 'grid' plot type.
(org-plot/gnuplot): Merge the parameters given in
`org-plot/preset-plot-types' and the #+PLOT line to ensure the former
is respected everywhere.

Reported-by: Visuwesh <visuweshm@gmail.com>
Link: https://orgmode.org/list/87edbu4kdh.fsf@gmail.com
---
 lisp/org-plot.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index 283d993..6d53830 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -349,7 +349,7 @@ (defcustom org-plot/preset-plot-types
     (grid :plot-cmd "splot"
 	  :plot-pre (lambda (_table _data-file _num-cols params _plot-str)
 		      (if (plist-get params :map) "set pm3d map" "set map"))
-	  :data-dump (lambda (table data-file params _num-cols)
+	  :data-dump (lambda (table data-file _num-cols params)
 		       (let ((y-labels (org-plot/gnuplot-to-grid-data
 					table data-file params)))
 			 (when y-labels (plist-put params :ylabels y-labels))))
@@ -391,8 +391,8 @@ (defcustom org-plot/preset-plot-types
 - :data-dump - Function to dump the table to a datafile for ease of
   use.
 
-  Accepts lambda function.  Default lambda body:
-  (org-plot/gnuplot-to-data table data-file params)
+  Accepts lambda function with arguments:
+  (table data-file num-cols params)
 
 - :plot-pre - Gnuplot code to be inserted early into the script, just
   after term and output have been set.
@@ -679,8 +679,8 @@ (defun org-plot/gnuplot (&optional params)
 		    tbl))
 	   (num-cols (length (if (eq (nth 0 table) 'hline) (nth 1 table)
 			       (nth 0 table))))
-	   (type (assoc (plist-get params :plot-type)
-			org-plot/preset-plot-types))
+	   (type (cdr (assoc (plist-get params :plot-type)
+			     org-plot/preset-plot-types)))
            gnuplot-script)
 
       (unless type
@@ -695,6 +695,7 @@ (defun org-plot/gnuplot (&optional params)
       (save-excursion (while (and (equal 0 (forward-line -1))
 				  (looking-at "[[:space:]]*#\\+"))
 			(setf params (org-plot/collect-options params))))
+      (setq params (org-combine-plists type params))
       ;; Dump table to datafile
       (let ((dump-func (plist-get type :data-dump)))
         (if dump-func
-- 
2.43.0

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

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

[வியாழன் ஜூன் 13, 2024] Visuwesh wrote:

> [புதன் ஜூன் 12, 2024] Ihor Radchenko wrote:
>
>> Visuwesh <visuweshm@gmail.com> writes:
>>
>>>> I do not think that it is right.
>>>> AFAIU, the idea is that `org-plot/preset-plot-types' provides some
>>>> default options, but the user can overwrite these defaults in the #+PLOT
>>>> line. What you propose will disregard the values of
>>>>
>>>>  :set :line :map :title :file :ind :timeind :timefmt :textind
>>>>  :deps :labels :xlabels :ylabels :xmin :xmax :ymin :ymax :ticks
>>>>
>>>> if they are customized by user in `org-plot/preset-plot-types'.
>>>
>>> I don't follow your conclusion since this change will only affect the
>>> value of :check-ind-type leaving the rest of the settings unaffected.
>>
>> My point is that we will eventually need to merge TYPE and PARAMS to
>> fix another bug - `org-plot/preset-plot-types' options like :set, :line,
>> etc being ignored. So, instead of patching the way you proposed, we can
>> merge TYPE and PARAMS into PARAMS, making the existing (plist-get params
>> :check-ind-type) working.
>>
>> In other words, org-plot's handling of parameters is very broken
>> now. There is more than one bug lurking there, and it may be more
>> productive to fix things together.
>>
>> You solution will, of course, work, but only for this specific bug you
>> described; not for other.
>
> OK, thanks for the explanation.  I was unaware of the issue as I didn't
> read the code closely enough.  What about the attached patch?

Sorry for the noise, I copied the wrong link in the commit message.
Please see attached instead.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-plot-Respect-parameters-given-in-org-plot-preset.patch --]
[-- Type: text/x-diff, Size: 2522 bytes --]

From 2a153a5bc015b0e970ecde39fd2edbb515261349 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Thu, 13 Jun 2024 12:33:49 +0530
Subject: [PATCH] org-plot: Respect parameters given in
 `org-plot/preset-plot-types'

* lisp/org-plot.el (org-plot/preset-plot-types): Fix docstring and
correct the lambda argument order for the 'grid' plot type.
(org-plot/gnuplot): Merge the parameters given in
`org-plot/preset-plot-types' and the #+PLOT line to ensure the former
is respected everywhere.

Reported-by: Visuwesh <visuweshm@gmail.com>
Link: https://orgmode.org/list/87cypbjw50.fsf@gmail.com
---
 lisp/org-plot.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index 283d993..6d53830 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -349,7 +349,7 @@ (defcustom org-plot/preset-plot-types
     (grid :plot-cmd "splot"
 	  :plot-pre (lambda (_table _data-file _num-cols params _plot-str)
 		      (if (plist-get params :map) "set pm3d map" "set map"))
-	  :data-dump (lambda (table data-file params _num-cols)
+	  :data-dump (lambda (table data-file _num-cols params)
 		       (let ((y-labels (org-plot/gnuplot-to-grid-data
 					table data-file params)))
 			 (when y-labels (plist-put params :ylabels y-labels))))
@@ -391,8 +391,8 @@ (defcustom org-plot/preset-plot-types
 - :data-dump - Function to dump the table to a datafile for ease of
   use.

-  Accepts lambda function.  Default lambda body:
-  (org-plot/gnuplot-to-data table data-file params)
+  Accepts lambda function with arguments:
+  (table data-file num-cols params)

 - :plot-pre - Gnuplot code to be inserted early into the script, just
   after term and output have been set.
@@ -679,8 +679,8 @@ (defun org-plot/gnuplot (&optional params)
 		    tbl))
 	   (num-cols (length (if (eq (nth 0 table) 'hline) (nth 1 table)
 			       (nth 0 table))))
-	   (type (assoc (plist-get params :plot-type)
-			org-plot/preset-plot-types))
+	   (type (cdr (assoc (plist-get params :plot-type)
+			     org-plot/preset-plot-types)))
            gnuplot-script)

       (unless type
@@ -695,6 +695,7 @@ (defun org-plot/gnuplot (&optional params)
       (save-excursion (while (and (equal 0 (forward-line -1))
 				  (looking-at "[[:space:]]*#\\+"))
 			(setf params (org-plot/collect-options params))))
+      (setq params (org-combine-plists type params))
       ;; Dump table to datafile
       (let ((dump-func (plist-get type :data-dump)))
         (if dump-func
--
2.43.0

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

> Sorry for the noise, I copied the wrong link in the commit message.
> Please see attached instead.

Thanks!
I have some comments.

> -	   (type (assoc (plist-get params :plot-type)
> -			org-plot/preset-plot-types))
> +	   (type (cdr (assoc (plist-get params :plot-type)
> +			     org-plot/preset-plot-types)))
>             gnuplot-script)

This may break the existing customization.
Later in the function, TYPE is used as an argument for
`org-plot/gnuplot-term-extra' and `org-plot/gnuplot-script-preamble'.
Some users may have these two custom options adjusted to the older
calling convention.

To not break things, we should pass the full `assoc' to these functions.

Also, while you are at it, may your please clarify what TYPE means in
the docstrings of `org-plot/gnuplot-term-extra' and
`org-plot/gnuplot-script-preamble'?

>  			(setf params (org-plot/collect-options params))))
> +      (setq params (org-combine-plists type params))

May you also drop a short comment in the code that explains what this
line does?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

[சனி ஜூன் 15, 2024] Ihor Radchenko wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> Sorry for the noise, I copied the wrong link in the commit message.
>> Please see attached instead.
>
> Thanks!
> I have some comments.
>
>> -	   (type (assoc (plist-get params :plot-type)
>> -			org-plot/preset-plot-types))
>> +	   (type (cdr (assoc (plist-get params :plot-type)
>> +			     org-plot/preset-plot-types)))
>>             gnuplot-script)
>
> This may break the existing customization.
> Later in the function, TYPE is used as an argument for
> `org-plot/gnuplot-term-extra' and `org-plot/gnuplot-script-preamble'.
> Some users may have these two custom options adjusted to the older
> calling convention.
>
> To not break things, we should pass the full `assoc' to these functions.

If you meant org-plot/gnuplot-script eventually calling these functions,
then I don't see how the above change will break things.  Even in
org-plot/gnuplot-script, TYPE passed to both these user options are

    (let* ((type-name (plist-get params :plot-type))
           (type (cdr (assoc type-name org-plot/preset-plot-types))))

so there should be no harm done by the above change since TYPE is not an
argument taken by org-plot/gnuplot-script.

> Also, while you are at it, may your please clarify what TYPE means in
> the docstrings of `org-plot/gnuplot-term-extra' and
> `org-plot/gnuplot-script-preamble'?

OK.

>>  			(setf params (org-plot/collect-options params))))
>> +      (setq params (org-combine-plists type params))
>
> May you also drop a short comment in the code that explains what this
> line does?

I will send an updated patch once my confusion is cleared.

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

>>> -	   (type (assoc (plist-get params :plot-type)
>>> -			org-plot/preset-plot-types))
>>> +	   (type (cdr (assoc (plist-get params :plot-type)
>>> +			     org-plot/preset-plot-types)))
>>>             gnuplot-script)
>>
>> This may break the existing customization.
>> Later in the function, TYPE is used as an argument for
>> `org-plot/gnuplot-term-extra' and `org-plot/gnuplot-script-preamble'.
>> Some users may have these two custom options adjusted to the older
>> calling convention.
>>
>> To not break things, we should pass the full `assoc' to these functions.
>
> If you meant org-plot/gnuplot-script eventually calling these functions,
> then I don't see how the above change will break things.  Even in
> org-plot/gnuplot-script, TYPE passed to both these user options are
>
>     (let* ((type-name (plist-get params :plot-type))
>            (type (cdr (assoc type-name org-plot/preset-plot-types))))
>
> so there should be no harm done by the above change since TYPE is not an
> argument taken by org-plot/gnuplot-script.

Agree. I accidentally moved away from the function that is actually
being changed to org-plot/gnuplot-script that has nothing to do with
TYPE binding you are changing in the patch.

So, your code is ok here.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Visuwesh]

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

[திங்கள் ஜூன் 17, 2024] Ihor Radchenko wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>>>> -	   (type (assoc (plist-get params :plot-type)
>>>> -			org-plot/preset-plot-types))
>>>> +	   (type (cdr (assoc (plist-get params :plot-type)
>>>> +			     org-plot/preset-plot-types)))
>>>>             gnuplot-script)
>>>
>>> This may break the existing customization.
>>> Later in the function, TYPE is used as an argument for
>>> `org-plot/gnuplot-term-extra' and `org-plot/gnuplot-script-preamble'.
>>> Some users may have these two custom options adjusted to the older
>>> calling convention.
>>>
>>> To not break things, we should pass the full `assoc' to these functions.
>>
>> If you meant org-plot/gnuplot-script eventually calling these functions,
>> then I don't see how the above change will break things.  Even in
>> org-plot/gnuplot-script, TYPE passed to both these user options are
>>
>>     (let* ((type-name (plist-get params :plot-type))
>>            (type (cdr (assoc type-name org-plot/preset-plot-types))))
>>
>> so there should be no harm done by the above change since TYPE is not an
>> argument taken by org-plot/gnuplot-script.
>
> Agree. I accidentally moved away from the function that is actually
> being changed to org-plot/gnuplot-script that has nothing to do with
> TYPE binding you are changing in the patch.
>
> So, your code is ok here.

Thanks, so please find attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-plot-Respect-parameters-given-in-org-plot-preset.patch --]
[-- Type: text/x-diff, Size: 4013 bytes --]

From 9755ccb636ab8e1d855bbc386bc6bab2203a2cd5 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Tue, 18 Jun 2024 10:39:27 +0530
Subject: [PATCH] org-plot: Respect parameters given in
 `org-plot/preset-plot-types'

* org-plot.el (org-plot/gnuplot-script-preamble)
(org-plot/gnuplot-term-extra): Explain what "plot type" means.
(org-plot/preset-plot-types): Fix docstring and correct the lambda
argument order for the 'grid' plot type.
(org-plot/gnuplot): Merge the parameters given in
`org-plot/preset-plot-types' and the #+PLOT line to ensure the former
is respected everywhere.

Reported-by: Visuwesh <visuweshm@gmail.com>
Link: https://orgmode.org/list/87cypbjw50.fsf@gmail.com
---
 lisp/org-plot.el | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/lisp/org-plot.el b/lisp/org-plot.el
index 83483b2..b436613 100644
--- a/lisp/org-plot.el
+++ b/lisp/org-plot.el
@@ -302,9 +302,10 @@ (defgroup org-plot nil
 (defcustom org-plot/gnuplot-script-preamble ""
   "String of function to be inserted before the gnuplot plot command is run.
 
-Note that this is in addition to, not instead of other content generated in
-`org-plot/gnuplot-script'.  If a function, it is called with the plot type as
-the argument, and must return a string to be used."
+Note that this is in addition to, not instead of other content generated
+in `org-plot/gnuplot-script'.  If a function, it is called with the
+parameters used by the current plot type (see
+`org-plot/preset-plot-types'), and must return a string to be used."
   :group 'org-plot
   :type '(choice string function))
 
@@ -349,7 +350,7 @@ (defcustom org-plot/preset-plot-types
     (grid :plot-cmd "splot"
 	  :plot-pre (lambda (_table _data-file _num-cols params _plot-str)
 		      (if (plist-get params :map) "set pm3d map" "set map"))
-	  :data-dump (lambda (table data-file params _num-cols)
+	  :data-dump (lambda (table data-file _num-cols params)
 		       (let ((y-labels (org-plot/gnuplot-to-grid-data
 					table data-file params)))
 			 (when y-labels (plist-put params :ylabels y-labels))))
@@ -391,8 +392,8 @@ (defcustom org-plot/preset-plot-types
 - :data-dump - Function to dump the table to a datafile for ease of
   use.
 
-  Accepts lambda function.  Default lambda body:
-  (org-plot/gnuplot-to-data table data-file params)
+  Accepts function with arguments:
+  (table data-file num-cols params)
 
 - :plot-pre - Gnuplot code to be inserted early into the script, just
   after term and output have been set.
@@ -541,7 +542,8 @@ (defcustom org-plot/gnuplot-term-extra ""
   "String or function which provides the extra term options.
 E.g. a value of \"size 1050,650\" would cause
 \"set term ... size 1050,650\" to be used.
-If a function, it is called with the plot type as the argument."
+If a function, it is called with the parameters used by the current plot
+type, see `org-plot/preset-plot-types'."
   :group 'org-plot
   :type '(choice string function))
 
@@ -678,8 +680,8 @@ (defun org-plot/gnuplot (&optional params)
 		    tbl))
 	   (num-cols (length (if (eq (nth 0 table) 'hline) (nth 1 table)
 			       (nth 0 table))))
-	   (type (assoc (plist-get params :plot-type)
-			org-plot/preset-plot-types))
+	   (type (cdr (assoc (plist-get params :plot-type)
+			     org-plot/preset-plot-types)))
            gnuplot-script data-file)
 
       (unless type
@@ -693,6 +695,10 @@ (defun org-plot/gnuplot (&optional params)
       (save-excursion (while (and (equal 0 (forward-line -1))
 				  (looking-at "[[:space:]]*#\\+"))
 			(setf params (org-plot/collect-options params))))
+      ;; Ensure that the user can override any plot parameter, and
+      ;; that the parameters set by the plot type in
+      ;; `org-plot/preset-plot-types' is respected.
+      (setq params (org-combine-plists type params))
       ;; Dump table to datafile
       (let ((dump-func (plist-get type :data-dump)))
         ;; Use a stable temporary file to ensure that 'replot' upon
-- 
2.43.0

Re: [BUG] org-plot: Unable to use text xtics with type:2d (+ more) [9.7-pre (N/A @ /home/viz/lib/emacs/straight/build/org/)]

[Ihor Radchenko]

Visuwesh <visuweshm@gmail.com> writes:

>> So, your code is ok here.
>
> Thanks, so please find attached.

Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fed19a934

Fixed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>