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

> 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 <>.
Support Org development at <>,
or support my work at <>

Reply via email to