Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Fri, Sep 15 2023, Ihor Radchenko <yantar92@posteo.net> wrote:

> Leo Butler <Leo.Butler@umanitoba.ca> writes:
>
>> Attached is a patch that tries to address some of Ihor's concerns. I
>> have added two header arguments for maxima src blocks:
>
> Thanks!
>
>> - :graphics-pkg lets the user choose the graphics package to use;
>>
>> - :batch lets the user choose which source-code loader Maxima will use.
>
> We need to document the new header arguments in ORG-NEWS and in
> https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-maxima.html

Thanks for your feedback.

Yes, I will do the documentation once the patch has attained a
nearer-to-finished state.

>
> Also, non-standard arguments should be defined in 
> `org-babel-header-args:maxima'.
>

Ok. I see that some packages (e.g. ob-gnuplot.el) use a `defvar' form,
while others (e.g. ob-R.el) use a `defconst' form. Is there a preference?

>> I have also moved two defaults, that were embedded in the code, to
>> `defvar' forms.
>
> This is fine, although I would prefer to keep these variables private for
> now.

My understanding is that a special variable defined via `defvar' is one
that is intended to be "private", i.e. users should not change it unless
they really know what they are doing. Is that accurate?

Do you mean the names should be something like

org-babel-maxima--command-arguments

?

>
>> +(defvar org-babel-maxima-command-arguments
>> +  "--very-quiet"
>> + "A string containing the command-line arguments used when calling
>> the Maxima executable. See `org-babel-maxima-command',
>> `org-babel-maxima-batch/load' and `org-babel-execute:maxima'.")
>
> Here and in the other docstrings, please use a single short sentence in
> the first line. Also, please use double space between sentences and fill
> the docstring text to standard fill-column. See
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html

Yes.

>
>> +(defvar org-babel-maxima-graphic-package-options
>> +  '((plot . "(set_plot_option ('[gnuplot_term, %s]), set_plot_option 
>> ('[gnuplot_out_file, %S]))$")
>> +    (draw . "(load(draw), set_draw_defaults(terminal='%s,file_name=%S))$"))
>> + "An alist, each element of the form (PACKAGE-NAME
>> . FORMAT-STRING). The format string contains the Maxima code to set
>> the graphic file terminal and name. It must contain `%s' to set the
>> terminal and `%S' to set the filename. The default package is
>> `plot'. See `org-babel-maxima-expand'.")
>
> According to the docstring, it appears that the order of %s and %S do
> not matter, which is probably not the case.

Correct.

>
>> +                  ;; graphic output
>> +                  (if graphic-file
>> +                          (let ((graphics-pkg (intern (or (cdr (assq 
>> :graphics-pkg params)) "plot")))
>> +                                (graphic-format-string (cdr (assq 
>> graphics-pkg org-babel-maxima-graphic-package-options)))
>> +                                (graphic-terminal (file-name-extension 
>> graphic-file))
>> +                                (graphic-file (if (eq graphics-pkg 'plot) 
>> graphic-file (file-name-sans-extension graphic-file))))
>> +                            (format graphic-format-string graphic-terminal 
>> graphic-file)))
>
> What will happen if :graphics-pkg value is not expected? What about
> unsupported graphics file extensions? 

I need to add a fail-safe when setting `graphic-format-string'

> And is it always the that terminal name is the same as file extension
> (I am thinking about Gnuplot's pngcairo terminal)?

I see your point.

At the moment, the code works for the following terminals:

png, jpg, gif, eps, svg, pdf

To support a range of terminal options including, I would need to add a
:terminal header argument and an alist of graphics packages and their
supported terminals.

>
>> -                              (format "batchload(%S)$" in-file))
>> +                              (format "(linenum:0, %s(%S))$" batch/load 
>> in-file))
>
> May you clarify the purpose of "linenum"?

Maxima keeps track of input/output line numbers via the variable
`linenum'. I set the linenum to 0 so that the line numbering of the
input in `in-file' starts at 1 not 2.

This idiom has been used in other Maxima front-ends, such as
`imaxima.el', too (although imaxima now uses the lisp reader, instead).

See 
https://sourceforge.net/p/maxima/code/ci/76105d9ee231679eccac888a04c98e6ef66df087/


>
>>                                (unless (or (string-match "batch" line)
>>                                            (string-match "^rat: replaced 
>> .*$" line)
>>                                            (string-match "^;;; Loading #P" 
>> line)
>> +                                          (string-match "^read and 
>> interpret" line)
>> +                                          (string-match 
>> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)
>
> May you explain why you added these two conditions?

When `batch' starts, it emits the line starting with 'read and
interpret' and including the name of the file being read. This is
extraneous output, and should be filtered out.

The second addition filters out empty input/output lines. Without that
filter, the block

#+name: ob-maxima/batch+verbatim+quiet
#+begin_src maxima :exports both :results verbatim :batch batch :cmdline --quiet
(assume(z>0),
integrate(exp(-t)*t^z, t, 0, inf));
#+end_src

produces an output with a pair of empty lines:

#+RESULTS: ob-maxima/batch+verbatim+quiet
: (%i1) (assume(z > 0),integrate(exp(-t)*t^z,t,0,inf))
: (%o1)                            gamma(z + 1)
: (%i2) 
: (%i4) 


I don't understand why those extra lines appear. It looks to me like a
bug in Maxima, but, because of that, they need to be filtered out.

----

If you recall, this thread started with the OP's request to replace
`batchload' with `batch' to support Maxima's :lisp reader. This patch
provides that support:

#+name: ob-maxima/batch+verbatim+:lisp
#+begin_src maxima :exports both :results verbatim :batch batch :cmdline --quiet
:lisp #$(assume(z>0),integrate(exp(-t)*t^z, t, 0, inf));#$
#+end_src

#+RESULTS: ob-maxima/batch+verbatim+:lisp
: ((%GAMMA SIMP) ((MPLUS SIMP) 1 $Z))

(The code may look like line noise, but what it does is it switches the
reader to read lisp code via :lisp, then it uses the lisp reader macro
#$ $# to read and evaluate Maxima code within the lisp reader. That
returns the lisp form of the result. These tricks are not possible using
`batchload'.)

I will prepare a revised patch based on your comments and answers to my
questions in this email.

TIA,
Leo