[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/)])

2023-09-05 Thread Ihor Radchenko
CCing Bastien, as he might want to intervene.

Leo Butler  writes:

   >> +(defcustom org-babel-maxima-command-arguments
   >> +  "--very-quiet"
   >
   >> +(defcustom org-babel-maxima-batch/load
   >> +  "batchload"
   >> +
   >> +(defcustom org-babel-maxima-graphic-file-format-string
   >> +  "(set_plot_option ('[gnuplot_term, png]), set_plot_option 
('[gnuplot_out_file, %S]))$"
   >> +
   >> +(defcustom org-babel-maxima-default-epilogue
   >> +  "gnuplot_close ()$"

>> IMHO, in their current state, if a user mindlessly customizes these
>> options without knowing how ob-maxima internals work, ob-maxima may
>> simply be broken.
>
> I think there is a fine line between being too rigid but working within
> a limited scope (as ob-maxima is now), or providing enough customizable
> options to let users do what they want. I would prefer the latter,
> if the defaults provide a working configuration.
>
> Note that I do attempt to suggest other working options in the defcustom
> definitions.

>> As a general rule, we do not expose internal details that are _required_
>> for things to work to users.
>
> I understand this principle, but, why not provide enough options for
> users to configure a package to do what they want? Yes, that may mean
> they break the package--but only temporarily, because returning to the
> default options will return the package to a working state.

That's a bit more tricky.
Imagine, for example, that we commit

+(defcustom org-babel-maxima-command-arguments
+  "--very-quiet"

and some users will later customize the default value to

"--very-quite --my-personal-switch-I-want"

Then, in future, due to changes in Org or maxima, we might need to
change "--very-quite" to something else in order to keep ob-maxima in
working condition: "--very-quite=yes 
--another-useful-flag-we-absolutely-need-in-org"

In order to make such a switch, we will have to force all the users
change their customization - something we do not want to annoy users
with.

So, leaving essential settings customizeable is not necessarily a good idea.

This is not necessarily a rigid rule though - we do leave, for example,
`org-latex-engraved-preamble' exposed to the users in ways that can
break LaTeX src block export. The basic condition is that changing
something should be practically useful for users, which is why I asked
you to elaborate on why each of the new customizations need to be
introduced.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-02 Thread Vladimir Nikishkin


Leo Butler  writes:

> On Sat, Sep 02 2023, Ihor Radchenko  wrote:
>
>> Lockywolf  writes:
>>
>>> At the moment, ob-maxima has a straightforward way of evaluating
>>> babel blocks,
>>>
>>> #+begin_src shell
>>> maxima --very-quiet -r batchload\(\"/tmp/ob-maximaFOOBAR.mac\"\)\$
>>> #+end_src
>>> (line 87 of ob-maxima.el),
>>>
>>> I suggest replacing batchload with batch(), and changing line 73 from
>>> "gnuplot_close ()$" to  "gnuplot_close ()$ \nquit();"
>>>
>>> The difference between "batch" and "batchload" is that "batch" can
>>> process :lisp expressions in addition to maxima's own, and it prints
>>> input/output labels. However, it is possible to customise label printing
>>> from maxima's own code, and being able to evaluate :lisp just seems
>>> uncontrovercially good.
>>>
>>> It might be that I am missing something, but batch seems a better fit
>>> for Org-Babel.
>>
>> May you please explain more about :lisp expressions?
>
> This special syntax is not necessary. A recent change introduced the
> function `eval_string_lisp' that removes the need for it. I.e. one can
> batchload a file and execute lisp code inside a call to eval_string_lisp.
>

Well, necessary or not, but it existed for many years, and continues to
be a valid part of maxima so far. It is surprising for new users to not
see it working when quite a lot of howtos use it. (Especially the famous
pattern-matching howto from Michael Talon.)

Also, I would be hesitant to say that it "removes the need for it",
because a "valid expression" is not the same thing as a "string". In
particular, error processing should be different.

>>
>> Also, what is the benefit/downside of printing input/output labels? Is
>> there any chance they will be caught into the source block output? If
>> so, it would be a breaking change.
>
> Yes, his requests are breaking changes. That is why I suggested opening
> up the internals a bit so that one can alter the behavior while
> maintaining the same default.
>
> Leo

Well, this change is "breaking", in the sense that it would make the
output of ob-maxima be more consistent with what, say, imaxima is doing.
However, I thing that there is a way to make it non-breaking, by
prepending some maxima code which would make adjust maxima using "batch"
to have the same output as "batchload". Something like
src_maxima{programmode: true; nolabels: true;} should turn off labels
if that is desired.

But I would agree with Leo Butler that customisation is the best
solution here. Both "full batch", and "pseudo-interactive" modes are
useful.

-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)



Re: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-02 Thread Leo Butler
On Sat, Sep 02 2023, Ihor Radchenko  wrote:

> Lockywolf  writes:
>
>> At the moment, ob-maxima has a straightforward way of evaluating
>> babel blocks,
>>
>> #+begin_src shell
>> maxima --very-quiet -r batchload\(\"/tmp/ob-maximaFOOBAR.mac\"\)\$
>> #+end_src
>> (line 87 of ob-maxima.el),
>>
>> I suggest replacing batchload with batch(), and changing line 73 from
>> "gnuplot_close ()$" to   "gnuplot_close ()$ \nquit();"
>>
>> The difference between "batch" and "batchload" is that "batch" can
>> process :lisp expressions in addition to maxima's own, and it prints
>> input/output labels. However, it is possible to customise label printing
>> from maxima's own code, and being able to evaluate :lisp just seems
>> uncontrovercially good.
>>
>> It might be that I am missing something, but batch seems a better fit
>> for Org-Babel.
>
> May you please explain more about :lisp expressions?

This special syntax is not necessary. A recent change introduced the
function `eval_string_lisp' that removes the need for it. I.e. one can
batchload a file and execute lisp code inside a call to eval_string_lisp.

>
> Also, what is the benefit/downside of printing input/output labels? Is
> there any chance they will be caught into the source block output? If
> so, it would be a breaking change.

Yes, his requests are breaking changes. That is why I suggested opening
up the internals a bit so that one can alter the behavior while
maintaining the same default.

Leo



Re: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-02 Thread Leo Butler
On Sat, Sep 02 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
>> I think that your request may be handled by one of two improvements:
>>
>> 1. Implement session support in ob-maxima.el; and
>> 2. Many of the design decisions in the existing ob-maxima code should be
>> customizable.
>
>> I am attaching a patch to address 2. Please try it out, I think that it
>> will satisfy your requests. Feedback is welcome.
>
> Thanks for the patch!
>
>> +(defcustom org-babel-maxima-command-arguments
>> +  "--very-quiet"
>
>> +(defcustom org-babel-maxima-batch/load
>> +  "batchload"
>> +
>> +(defcustom org-babel-maxima-graphic-file-format-string
>> +  "(set_plot_option ('[gnuplot_term, png]), set_plot_option 
>> ('[gnuplot_out_file, %S]))$"
>> +
>> +(defcustom org-babel-maxima-default-epilogue
>> +  "gnuplot_close ()$"
>
> This might be fine, but may you please explain what would be the purpose
> of customizing each of these options?

I am not sure of your request: do you want me to elaborate in the
docstrings? Or here?

>
> IMHO, in their current state, if a user mindlessly customizes these
> options without knowing how ob-maxima internals work, ob-maxima may
> simply be broken.

I think there is a fine line between being too rigid but working within
a limited scope (as ob-maxima is now), or providing enough customizable
options to let users do what they want. I would prefer the latter,
if the defaults provide a working configuration.

Note that I do attempt to suggest other working options in the defcustom
definitions.

> It is also not very clear what would be the benefit
> customizing any of the proposed options.

I had planned to update the worg documentation with some examples, but
here is one that answers the OP's original question:

>> +(defcustom org-babel-maxima-command-arguments
>> +  "--very-quiet"

This is the current setting.

It can be useful to include the build information that Maxima provides
on start-up, which would mean setting this to "".

>
>> +(defcustom org-babel-maxima-batch/load
>> +  "batchload"

This is the current setting.

Another option suggested is "batch". That is what the OP originally
requested.

>> +
>> +(defcustom org-babel-maxima-graphic-file-format-string
>> +  "(set_plot_option ('[gnuplot_term, png]), set_plot_option 
>> ('[gnuplot_out_file, %S]))$"

This is the current setting.

An alternative is to use the `draw' package, which is a suggested
option. Draw, in my opinion, is a much better package for plotting.

>> +
>> +(defcustom org-babel-maxima-default-epilogue
>> +  "gnuplot_close ()$"

This is the current setting.

The current setting is not needed in recent versions of Maxima (since
roughly 2010, I believe), except possibly on Windows. And, it is only
needed if `:results graphics file' is included as a src header.

On the other hand, the OP had requested adding

"quit() $"

to this setting. I don't think that is needed, but Maxima is built with
more than 6 different common-lisp implementations so there can be lots
of variability in how common tasks are performed...

>
> As a general rule, we do not expose internal details that are _required_
> for things to work to users.

I understand this principle, but, why not provide enough options for
users to configure a package to do what they want? Yes, that may mean
they break the package--but only temporarily, because returning to the
default options will return the package to a working state.

> In the above, `org-babel-maxima-default-epilogue' appears to be paired
> with `org-babel-maxima-graphic-file-format-string' and may need to be
> changed depending on its value.

No, they are not closely connected. Both plot and draw use gnuplot as
the graphing backend (although draw might be able to use vtk, that
capability has not been maintained...).

> Also, I am not sure if removing --very-quiet may
> not affect :results output.

Yes, it will. But that is what the OP is asking for.



I think that the original intention of ob-maxima was to provide a simple
`calculator'-like interface to Maxima. In many cases, though, one wants
to show a sequence of calculations that lead to the final answer. That
is why being able to configure some of these hard-coded settings is
desirable: to be able to modify how the input and output are presented.

Best regards,
Leo


Re: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-02 Thread Ihor Radchenko
Leo Butler  writes:

> I think that your request may be handled by one of two improvements:
>
> 1. Implement session support in ob-maxima.el; and
> 2. Many of the design decisions in the existing ob-maxima code should be
> customizable.

> I am attaching a patch to address 2. Please try it out, I think that it
> will satisfy your requests. Feedback is welcome.

Thanks for the patch!

> +(defcustom org-babel-maxima-command-arguments
> +  "--very-quiet"

> +(defcustom org-babel-maxima-batch/load
> +  "batchload"
> +
> +(defcustom org-babel-maxima-graphic-file-format-string
> +  "(set_plot_option ('[gnuplot_term, png]), set_plot_option 
> ('[gnuplot_out_file, %S]))$"
> +
> +(defcustom org-babel-maxima-default-epilogue
> +  "gnuplot_close ()$"

This might be fine, but may you please explain what would be the purpose
of customizing each of these options?

IMHO, in their current state, if a user mindlessly customizes these
options without knowing how ob-maxima internals work, ob-maxima may
simply be broken. It is also not very clear what would be the benefit
customizing any of the proposed options.

As a general rule, we do not expose internal details that are _required_
for things to work to users. In the above,
`org-babel-maxima-default-epilogue' appears to be paired with
`org-babel-maxima-graphic-file-format-string' and may need to be changed
depending on its value. Also, I am not sure if removing --very-quiet may
not affect :results output.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-02 Thread Ihor Radchenko
Lockywolf  writes:

> At the moment, ob-maxima has a straightforward way of evaluating
> babel blocks,
>
> #+begin_src shell
> maxima --very-quiet -r batchload\(\"/tmp/ob-maximaFOOBAR.mac\"\)\$
> #+end_src
> (line 87 of ob-maxima.el),
>
> I suggest replacing batchload with batch(), and changing line 73 from
> "gnuplot_close ()$" to"gnuplot_close ()$ \nquit();"
>
> The difference between "batch" and "batchload" is that "batch" can
> process :lisp expressions in addition to maxima's own, and it prints
> input/output labels. However, it is possible to customise label printing
> from maxima's own code, and being able to evaluate :lisp just seems
> uncontrovercially good.
>
> It might be that I am missing something, but batch seems a better fit
> for Org-Babel.

May you please explain more about :lisp expressions?

Also, what is the benefit/downside of printing input/output labels? Is
there any chance they will be caught into the source block output? If
so, it would be a breaking change.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-01 Thread Leo Butler
On Fri, Sep 01 2023, Lockywolf  wrote:

> Dear org developers,
>
> At the moment, ob-maxima has a straightforward way of evaluating
> babel blocks,
>
> #+begin_src shell
> maxima --very-quiet -r batchload\(\"/tmp/ob-maximaFOOBAR.mac\"\)\$
> #+end_src
>
> (line 87 of ob-maxima.el),
>
> I suggest replacing batchload with batch(), and changing line 73 from
> "gnuplot_close ()$" to"gnuplot_close ()$ \nquit();"
>
> The difference between "batch" and "batchload" is that "batch" can
> process :lisp expressions in addition to maxima's own, and it prints
> input/output labels. However, it is possible to customise label printing
> from maxima's own code, and being able to evaluate :lisp just seems
> uncontrovercially good.
>
> It might be that I am missing something, but batch seems a better fit
> for Org-Babel.

Hello,

I think that your request may be handled by one of two improvements:

1. Implement session support in ob-maxima.el; and
2. Many of the design decisions in the existing ob-maxima code should be
customizable.

I am attaching a patch to address 2. Please try it out, I think that it
will satisfy your requests. Feedback is welcome.

---

This old thread may also be relevant:
https://list.orgmode.org/87o7q5rw62@t14.reltub.ca/

Best,
Leo


diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index d1d7c7424..848811628 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -48,6 +48,33 @@
   :group 'org-babel
   :type 'string)
 
+(defcustom org-babel-maxima-command-arguments
+  "--very-quiet"
+  "Command-line arguments used when calling the Maxima executable. See `org-babel-maxima-batch/load' and `org-babel-execute:maxima'."
+  :group 'org-babel
+  :type 'string)
+
+(defcustom org-babel-maxima-batch/load
+  "batchload"
+  "The Maxima function used to read and execute Maxima code: `batchload' and `batch' are two alternatives, although a user-defined Maxima function may also be used. See `org-babel-execute:maxima'."
+  :options '("batchload" "batch")
+  :group 'org-babel
+  :type 'string)
+
+(defcustom org-babel-maxima-graphic-file-format-string
+  "(set_plot_option ('[gnuplot_term, png]), set_plot_option ('[gnuplot_out_file, %S]))$"
+  "A string with the Maxima code to set the graphic file terminal and name. It must contain `%S' to set the filename. See `org-babel-maxima-expand'."
+  :options '("(set_plot_option ('[gnuplot_term, png]), set_plot_option ('[gnuplot_out_file, %S]))$" "(load(draw), set_draw_option(terminal='pngcairo,file_name=%S))$")
+  :group 'org-babel
+  :type 'string)
+
+
+(defcustom org-babel-maxima-default-epilogue
+  "gnuplot_close ()$"
+  "A string with the final Maxima code executed. See `org-babel-maxima-expand'."
+  :group 'org-babel
+  :type 'string)
+
 (defun org-babel-maxima-expand (body params)
   "Expand a block of Maxima code according to its header arguments."
   (let ((vars (org-babel--get-vars params))
@@ -60,9 +87,7 @@
 		;; graphic output
 		(let ((graphic-file (ignore-errors (org-babel-graphical-output-file params
 		  (if graphic-file
-		  (format
-		   "set_plot_option ([gnuplot_term, png]); set_plot_option ([gnuplot_out_file, %S]);"
-		   graphic-file)
+		  (format org-babel-maxima-graphic-file-format-string graphic-file)
 		""))
 		;; variables
 		(mapconcat 'org-babel-maxima-var-to-maxima vars "\n")
@@ -70,7 +95,7 @@
 		body
 		;; Any code from the specified epilogue at the end.
 		epilogue
-		"gnuplot_close ()$")
+		org-babel-maxima-default-epilogue)
 	   "\n")))
 
 (defun org-babel-execute:maxima (body params)
@@ -81,10 +106,11 @@ This function is called by `org-babel-execute-src-block'."
 	(result
 	 (let* ((cmdline (or (cdr (assq :cmdline params)) ""))
 		(in-file (org-babel-temp-file "maxima-" ".max"))
-		(cmd (format "%s --very-quiet -r %s %s"
+		(cmd (format "%s %s -r %s %s"
 			 org-babel-maxima-command
+ org-babel-maxima-command-arguments
  (shell-quote-argument
-  (format "batchload(%S)$" in-file))
+  (format "%s(%S)$" org-babel-maxima-batch/load in-file))
  cmdline)))
 	   (with-temp-file in-file (insert (org-babel-maxima-expand body params)))
 	   (message cmd)


[BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]

2023-09-01 Thread Lockywolf



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.


Dear org developers,

At the moment, ob-maxima has a straightforward way of evaluating
babel blocks,

#+begin_src shell
maxima --very-quiet -r batchload\(\"/tmp/ob-maximaFOOBAR.mac\"\)\$
#+end_src
(line 87 of ob-maxima.el),

I suggest replacing batchload with batch(), and changing line 73 from
"gnuplot_close ()$" to  "gnuplot_close ()$ \nquit();"

The difference between "batch" and "batchload" is that "batch" can
process :lisp expressions in addition to maxima's own, and it prints
input/output labels. However, it is possible to customise label printing
from maxima's own code, and being able to evaluate :lisp just seems
uncontrovercially good.

It might be that I am missing something, but batch seems a better fit
for Org-Babel.

Emacs  : GNU Emacs 30.0.50 (build 1, x86_64-slackware-linux-gnu, GTK+ Version 
3.24.31, cairo version 1.16.0)
 of 2023-07-31
Package: Org mode version 9.6.6 (release_9.6.6 @ 
/usr/share/emacs/30.0.50/lisp/org/)
-- 
Your sincerely,
Vladimir Nikishkin (MiEr, lockywolf)
(Laptop)