emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Leo Butler <Leo.Butler@umanitoba.ca>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: Bastien <bzg@gnu.org>,
	Lockywolf <for_org-bugs_2023-09-01@lockywolf.net>,
	 "emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)
Date: Fri, 15 Sep 2023 15:13:47 +0000	[thread overview]
Message-ID: <87ediz4ggm.fsf@t14.reltub.ca> (raw)
In-Reply-To: <87jzsrai3x.fsf@localhost> (Ihor Radchenko's message of "Fri, 15 Sep 2023 09:41:38 +0000")

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

  reply	other threads:[~2023-09-15 15:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-01  4:35 [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)] Lockywolf
2023-09-01 18:33 ` Leo Butler
2023-09-02  7:19   ` Ihor Radchenko
2023-09-02 18:12     ` Leo Butler
2023-09-05 10:57       ` [MAINTENANCE] On how much we can expose internals into defcustom (was: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]) Ihor Radchenko
2023-09-06 19:39         ` [MAINTENANCE] On how much we can expose internals into defcustom Leo Butler
2023-09-07 11:35           ` Ihor Radchenko
2023-09-12 21:09             ` [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom) Leo Butler
2023-09-15  9:41               ` Ihor Radchenko
2023-09-15 15:13                 ` Leo Butler [this message]
2023-09-16  9:04                   ` Ihor Radchenko
2023-09-19 19:25                     ` Leo Butler
2023-09-20  9:17                       ` Ihor Radchenko
2023-09-20 15:02                         ` Leo Butler
2023-09-21  9:18                           ` Ihor Radchenko
2023-09-21 14:03                             ` Leo Butler
2023-09-22  9:43                               ` Ihor Radchenko
2023-10-02 16:01                                 ` Leo Butler
2023-10-04  8:38                                   ` Ihor Radchenko
2023-10-04 13:07                                     ` Leo Butler
2023-09-02  7:06 ` [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)] Ihor Radchenko
2023-09-02 18:20   ` Leo Butler
2023-09-03  5:25     ` Vladimir Nikishkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ediz4ggm.fsf@t14.reltub.ca \
    --to=leo.butler@umanitoba.ca \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=for_org-bugs_2023-09-01@lockywolf.net \
    --cc=yantar92@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).