Re: ob-octave: improve MATLAB support

[John C <john.ciolfi.32@gmail.com>]

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

Hi

Thanks for the feedback. See updated org-matlab.patch. Note code evaluation
will now error out when using an out-of-date matlab-mode (rather than
generating a warning) because when using an older matlab-mode, code block
evaluation doesn't work. We need to wait for the matlab-shell to be ready.
Without that capability found in newer matlab-mode, the errors are very
hard to understand. I also added tests. Using steps outlined in the
ob-matlab-test.el comment to setup for MATLAB testing, I get on Linux:

matlab-mode, version 6.3
Detected MATLAB R2024b (24.2)  -- Loading history file
matlab-shell: starting server with name server-2055614
   passed   127/1250  ob-matlab/results-file-graphics (9.712776 sec)
   passed   128/1250  ob-matlab/results-file-graphics-with-space (0.332004
sec)
   passed   129/1250  ob-matlab/results-output (0.013681 sec)
   passed   130/1250  ob-matlab/results-output-latex (0.459980 sec)
   passed   131/1250  ob-matlab/results-output-preserve-whitespace
(0.281001 sec)
   passed   132/1250  ob-matlab/results-output-reuse-a (0.039130 sec)
   passed   133/1250  ob-matlab/results-output-reuse-b (0.043762 sec)
   passed   134/1250  ob-matlab/results-output-reuse-clear (0.301597 sec)
   passed   135/1250  ob-matlab/results-verbatim (0.172567 sec)

You mentioned that we should expect org users to understand sessions,
however the doc on sessions is a bit light. Searching
https://orgmode.org/org.html for "session", we see it used two different
ways "Emacs session" and babel code block evaluations sessions described in
"Using sessions". What is there is okay, but doesn't really help one
understand when they should/shouldn't use babel code block evaluation
sessions. What would be nice is if "Using sessions" were titled "Babel code
block evaluation sessions" containing the existing content minus the "Only
languages that provide interactive evaluation ..." paragraph because this
is vague. In addition, each language should have a way of describing what
:session means to them, perhaps by extracting this info from ob-LANGUAGE.el
or maybe ob-LANGUAGE.org, or maybe something else that inserts the language
specific into the org manual.

With good MATLAB integration in org-mode, we'll see scientists and
engineers who have limited programming background leverage org-mode for
their scientific papers. Not having to worry about more advanced concepts
like sessions is nice, so thanks for letting us default the :session for
MATLAB to make it work as one would expect.

Here's the commit info:

ob-matlab.el: improve MATLAB support

* lisp/ob-matlab.el (header): Update URL for MATLAB

* lisp/ob-octave.el (org-babel-octave-evaluate): Fixed MATLAB support
  - Deprecate variables related to MATLAB Emacs Link and removed the code.
    Emacs Link capability was removed from MATLAB release R2009a, 15 years
ago.
  - Fixed the following type of org block evaluation:
    1) #+begin_src matlab :results verbatim
    2) #+begin_src matlab :results output
    3) #+begin_src matlab :results output latex
    4) #+begin_src matlab :results file graphics
    which aid in writing scientific papers.
  - Minor point, the correct spelling of MATLAB when referencing the
product is
    all upper case.

* lisp/ob-comint.el: enhanced org-babel-comint-with-output
  - The META argument of org-babel-comint-with-output now supports an
optional
    STRIP-REGEXPS which can be used to remove content from the returned
output.

* lisp/org.el
  - Add MATLAB as one of the options for org-babel-load-languages

* mk/default.mk
  - Add support for testing matlab code blocks.

* testing/examples/ob-matlab-test.org, testing/lisp/test-ob-matlab.el
  - Test for matlab code block.

* testing/org-test.el
  - Added org-test-get-code-block which is for use by testing/lisp/test*.el
files
    to extract code blocks from testing/examples/*.org files for on-the-fly
    testing using org-test-with-temp-text.

* etc/ORG-NEWS (New functions and changes in function arguments):
  Added entry "ob-octave: improved MATLAB support"

Thanks
John


On Sun, Dec 29, 2024 at 2:41 AM Ihor Radchenko <yantar92@posteo.net> wrote:

> John C <john.ciolfi.32@gmail.com> writes:
>
> > See attached org-matlab.patch which addresses all feedback. Here's the
> > commit info.
>
> Thanks!
> See my comments inline.
>
> > +*** ob-matlab: fixed MATLAB support
> > +
> > +Fixed MATLAB babel code blocks processing. MATLAB code blocks,
> ~#+begin_src matlab~, with ~:results
> > +verbatim~, ~:results output~, ~:results output latex~, or ~:results
> file graphics~ now work.  Fixes
> > +include (1) waiting for matlab-shell to start before evaluating MATLAB
> code, (2) correctly showing
> > +the results using writematrix, (3) removing the code block lines from
> the result, (4) correctly
> > +handling graphics results by invoking print correctly. To use MATLAB
> with org, you need
> > +https://github.com/MathWorks/Emacs-MATLAB-Mode.
>
> There is no need to provide so many details.
> Just leave the most important things:
>
> 1. MATLAB is no longer broken
> 2. Emacs-MATLAB-Mode is required now
>
> > +*** ob-matlab: MATLAB behavior change
> > +
> > +MATLAB code blocks now reuse the ~MATLAB*~ buffer created by ~M-x
> matlab-shell~, whereas the
> > +prior version started a new shell for each evaluation.  The benefit of
> this is that
> > +evaluations are very fast after the first evaluation and that state is
> maintained between
> > +evaluations, which you can clear using the MATLAB ~clear~ command.
> Another benefit of this
> > +behavior is that it is consistent with how MATLAB works.
>
> No need to explain in so much details, I think.
> Just say that MATLAB uses session by default and them mention that users
> may customize `org-babel-default-header-args:matlab' to disable session.
>
> > +(defun org-babel-comint--strip-regexps (result strip-regexps)
> > +  "STRIP-REGEXPS from RESULT list of strings."
> > +  (dolist (strip-regexp strip-regexps)
> > +    (let ((new-result '()))
> > +      (dolist (line result)
> > +        (setq line (replace-regexp-in-string strip-regexp "" line))
> > +        (when (not (string= line ""))
> > +          (setq new-result (append new-result `(,line)))))
>
> It is more efficient to use `push' + `nreverse' instead of `append'.
>
> > -(defvar org-babel-default-header-args:matlab '())
> > +;; With `org-babel-default-header-args:matlab' set to
> > +;;  '((:session . "*MATLAB*")))
> > +;; ...
> > +;; If you want a new session each time you evaluate a MATLAB code block,
> > +;;   (setq 'org-babel-default-header-args:matlab '())
> > +;; However, this will make evaluations slower and is not consistent
> with how
> > +;; MATLAB works.  MATLAB is designed for many evaluations.
> > +(defvar org-babel-default-header-args:matlab '((:session . "*MATLAB*")))
>
> You don't need that long comment in the source code.
> If you think that explaining the details about session is necessary (it
> may or may not be, but we should assume that Org users are familiar with
> the notion of sessions in code blocks), please do it in the
> documentation, not in the code.
>
> More generally, your motivation is not specific to matlab. Yet, we
> default to no session in most babel backends. So, it is not a question
> of session being faster or slower, but a question of consistency.
>
> That said, some babel backends do default to session, so I do not oppose
> this change too much.
>
> > +(make-obsolete-variable 'org-babel-matlab-with-emacs-link
> > +                        "MATLAB removed EmacsLink in R2009a." "2009")
> > +
> > +(make-obsolete-variable 'org-babel-matlab-emacs-link-wrapper-method
> > +                        "MATLAB removed EmacsLink in R2009a." "2009")
>
> Please use Org version in WHEN argument of `make-obsolete-variable'.
> The WHEN should be "9.8".
>
> > +(defun org-babel-matlab-shell ()
> > +  "Start and/or wait for MATLAB shell."
> > +  (require 'matlab-shell) ;; make `matlab-shell-busy-checker' available
> > +  (cond
> > +   ((fboundp 'matlab-shell-busy-checker)
> > +    ;; Start the shell if needed.  `matlab-shell' will reuse existing
> if already running.
> > +    (matlab-shell)
> > +    ;; If we just started the matlab-shell, wait for the prompt.  If we
> do not
> > +    ;; wait, then the startup messages will show up in the evaluation
> results.
> > +    (matlab-shell-busy-checker 'wait-for-prompt))
> > +   (t
> > +    (message (concat "You version of matlab-mode is old.\n"
> > +                     "Please update, see
> https://github.com/mathworks/Emacs-MATLAB-Mode\n"
> > +                     "Updating will eliminate unexpected output in your
> results\n"))
> > +    (sit-for 3)
>
> Instead of `message' + `sit-fit', you can simply use `display-warning'.
> It will give users more control.
>
> > +(defun org-babel-body-for-output (body matlabp)
> > +  "If MATLABP, fixup BODY for MATLAB output result-type."
> > +  (when matlabp
> > +    ;; When we send multi-line input to `matlab-shell', we'll see the
> "body"
> > +    ;; code lines echoed in the output which is not what one would
> expect.  To
> > +    ;; remove these unwanted lines, we append a comment "%-<org-eval>"
> to each
> > +    ;; line in the body MATLAB code.  After we collect the results from
> > +    ;; evaluation, we leverage the "%-<org-eval>" to remove the
> unwanted lines.
> > +    ;; Example of desired behavior:
> > ...
>
> I think that an important point here is that MATLAB does not echo the
> whole BODY all at once and instead mixes it with the output. Which is
> why we need to do something non-standard to filter out the body in
> matlab specifically.
>
> > +    (setq body (replace-regexp-in-string "\n" " %-<org-eval>-\n" body))
> > +    (when (not (string-match "\n\\'" body))
> > +      (setq body (concat body " %-<org-eval>-"))))
> > +  body)
>
> Please put this %-<org-eval> into an internal constant and then reuse
> it when building the regexp to filter.
>
> > +               (when matlabp
> > +                 '(;; MATLAB echo's all input lines, so use the
> %-<org-eval> comments to strip
> > +                   ;; them from the output
> > +                   "^[^\n]*%-<org-eval>-\n"
> > +                   ;; Remove starting blank line caused by stripping
> %-<org-eval>
> > +                   "\\`[[:space:]\r\n]+"
> > +                   ;; Strip <ERRORTXT> and </ERRORTXT> matlab-shell
> error indicators
> > +                   "</?ERRORTXT>\n")))
>
> Same here. Please put these regexps into a constant.
>
> --
> Ihor Radchenko // yantar92,
> Org mode maintainer,
> 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>
>

[-- Attachment #2: Type: text/html, Size: 13402 bytes --]