Make maxima(<string>) output parseable

[EmmanuelCharpentier]

(See also this sage-devel post.)

Currently, all whitespace is stripped off maxima(x) and maxima_calculus(x) where x is a string. The same goes for Maxima code defined via the interfaces. To illustrate :

r1=maxima_calculus('foo: a and (b or c)')
r2=maxima_calculus('untree(x) := append([part(x, 0)], maplist(lambda([u], if atom(u) then u else untree(u)), x))')
r3=maxima_calculus('untree(foo)')
(r1, r2, r3)

outputs :

(aand(borc),
 untree(x):=append([part(x,0)],maplist(lambda([u],ifatom(u)thenuelseuntree(u)),x)),
 [\"and\",a,[\"or\",b,c]])

This also deminstrates that the Maxima interpretation of the first instruction is correct ; it is its output which is ill-interpreted by Sage...

In order to maintain the print(eval(read(x))==x equality, it is desirable to keep spaces at least around boolean operators.

CC: @fchapoton @nbruin @slel @spaghettisalat

Component: interfaces

Keywords: maxima symbolics

Author: Emmanuel Charpentier, Matthias Koeppe, Frédéric Chapoton

Branch/Commit: 05fa319

Reviewer: Samuel Lelièvre

Issue created by migration from https://trac.sagemath.org/ticket/31796

[nbruin]

comment:1

It actually looks like whitespace has been stripped right from the start from output in the maxima interface:
https://github.com/sagemath/sage-prod/blob/e76b97fedaaec412fb4cce98bc55eb3d0ba2697f/src/sage/interfaces/maxima.py#L562
(the blame of that line goes back to original commits)

If I recall correctly, there were problems with maxima inserting spaces at unpredictable places every now and again; even in the middle of identifiers! The original commits would have been for maxima on CLISP rather than on ECL (although I recall the whitespace problem occurring on ECL (too?)). It could be that with the different iterations of "expect" -- and almost certainly in maxima_lib -- this isn't a problem anymore, but it means you'll have to test a lot in order to make really sure that a very old problem doesn't inadvertently comes back.

Also note that maxima_lib and maxima each have their own _eval_line_ which does the stripping, so to keep the two interfaces in step, you'd want to change it in both.

[EmmanuelCharpentier]

Branch: u/charpent/legible_maxima_output

[EmmanuelCharpentier]

Commit: 5f31eeb

[EmmanuelCharpentier]

comment:3

Replying to @nbruin:

It actually looks like whitespace has been stripped right from the start from output in the maxima interface:
https://github.com/sagemath/sage-prod/blob/e76b97fedaaec412fb4cce98bc55eb3d0ba2697f/src/sage/interfaces/maxima.py#L562
(the blame of that line goes back to original commits)

If I recall correctly, there were problems with maxima inserting spaces at unpredictable places every now and again;

This still happens, at least for MaximaElement pieces. To be explored in a followup ticket (but I'm afraid that this one is in Maxima's territory...). I've installed a workaround for this in assumptions.py

even in the middle of identifiers!

This I didn't see (yet).

The original commits would have been for maxima on CLISP rather than on ECL (although I recall the whitespace problem occurring on ECL (too?)). It could be that with the different iterations of "expect" -- and almost certainly in maxima_lib -- this isn't a problem anymore, but it means you'll have to test a lot in order to make really sure that a very old problem doesn't inadvertently comes back.

Advice gladly accepted for filing and solving a followup ticket, or to better test the proposed solution.

For the latter : what would you think of generating random SR expressions, translate them to Maxima strings, and check back-and-forth translations ? ISTR that such a random expression generator exists somewhere in Sage but, for the life of me, I'm unable to recall where I saw this...

Also note that maxima_lib and maxima each have their own _eval_line_ which does the stripping,

In different ways ==> two different fixes. No possible code sharing here without major restructuration.

so to keep the two interfaces in step, you'd want to change it in both.

Done. You're welcome to review my branch, which passes ptestlong with exactly the same errors as develop at 9.3.rc5 (and therefore needs_review). And delivers parseable output for Maxima strings containing logical operators. Yay !

---

New commits:

5f31eeb

Ticket #31796 : make maxima output parseable.

[EmmanuelCharpentier]

comment:4

Note : Lint complains concern exclusively trailing whitespace (a couple occurrences in my patch) and multiple statements on one line (none in my patch).

Should I try and fix all these occurrences (at least as a public service) ? Or should this be a separate ticket ?

[slel]

comment:5

There seems to be a copy-paste mistake in the German
and Japanese tour_algebra.rst, with the de2 block
replaced by the adapted de1 block rather than by the
adapted de2 block.

[slel]

Reviewer: Samuel Lelièvre

[slel]

comment:6

Please fill in author name so patchbots run.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

d2059f7

Ticket #31796: fix paste mistakes in german and japanese tutorials.

[sagetrac-git]

Changed commit from 5f31eeb to d2059f7

[EmmanuelCharpentier]

comment:8

Replying to @slel:

There seems to be a copy-paste mistake in the German
and Japanese tour_algebra.rst, with the de2 block
replaced by the adapted de1 block rather than by the
adapted de2 block.

Right. Damn... Fixed.

[EmmanuelCharpentier]

comment:9

Replying to @slel:

Please fill in author name so patchbots run.

Done. Thank you !

[EmmanuelCharpentier]

Author: Emmanuel Charpentier

[slel]

comment:11

The patchbot's pyflakes plugin complains:

- src/sage/interfaces/maxima.py:498:1
  'sage.env.DOT_SAGE' imported but unused

- src/sage/interfaces/maxima.py:498:1
  'sage.env.SAGE_LOCAL' imported but unused

- src/sage/interfaces/maxima_abstract.py:997:9
  local variable 'a' is assigned to but never used

- src/sage/symbolic/expression_conversions.py:21:1
  'sage.symbolic.constants.I' imported but unused

- src/sage/symbolic/expression_conversions.py:1112:13
  redefinition of unused 'I' from line 21

Not sure what to make of those, especially the first two.

Someone with an expert opinion on those please chime in.

[EmmanuelCharpentier]

comment:12

Replying to @slel:

The patchbot's pyflakes plugin complains:

- src/sage/interfaces/maxima.py:498:1
  'sage.env.DOT_SAGE' imported but unused

- src/sage/interfaces/maxima.py:498:1
  'sage.env.SAGE_LOCAL' imported but unused

Indeed : neither DOT_SAGE nor SAGE_LOCAL appear anywhere in this file but on line 487, where they are imported from sage.env.

Residues of code past ?

- src/sage/interfaces/maxima_abstract.py:997:9
  local variable 'a' is assigned to but never used}}}

This one is right, but seems harmless. Was in the original code.

- src/sage/symbolic/expression_conversions.py:21:1
  'sage.symbolic.constants.I' imported but unused

- src/sage/symbolic/expression_conversions.py:1112:13
  redefinition of unused 'I' from line 21

This one is hard to check. After hunting all capital I in the file :

• In live code, I find an import of I in AlgebraicConverter.arithmetic, line 1112 (as announced by pyflakes).
• All the other uses are in examples/tests.

Past code residues, again ?

Not sure what to make of those, especially the first two.

Someone with an expert opinion on those please chime in.

I'm interested, too.

But I can also make a new commit including the changes suggested by pyflakes (as well as deleting the "trailing whitespace" relint complains about so insistently, but not the semicolon uses pycodestyle hates...).

Advice ?

[slel]

comment:13

Let us get this in; pyflakes and relint
can be worked on in a follow-up ticket.

[vbraun]

comment:14

The ticket tests fine, but once you remove the $DOT_SAGE/maxima_commandlist_cache.sobj then tests fail with (this took quite a long time to track down):

sage -t --long --random-seed=0 src/sage/symbolic/maxima_wrapper.py
**********************************************************************
File "src/sage/symbolic/maxima_wrapper.py", line 81, in sage.symbolic.maxima_wrapper.MaximaWrapper.__getattr__
Failed example:
    s.completions('u.airy_',globals())
Expected:
    ['u.airy_ai', 'u.airy_bi', 'u.airy_dai', 'u.airy_dbi']
Got:
    ['u.airy_ai', 'u.airy_bi', 'u.airy_dbi']
**********************************************************************
1 item had failures:
   1 of   7 in sage.symbolic.maxima_wrapper.MaximaWrapper.__getattr__
    [30 tests, 1 failure, 1.40 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/symbolic/maxima_wrapper.py  # 1 doctest failed
----------------------------------------------------------------------

How about we just version it as maxima_commandlist_cache_v2.sobj

[mkoeppe]

Changed branch from u/charpent/legible_maxima_output to u/mkoeppe/legible_maxima_output

[mkoeppe]

Changed commit from d2059f7 to a3aa3c9

[mkoeppe]

comment:19

rebased on 9.4.rc2

---

New commits:

a3aa3c9

Ticket #31796 : make maxima output parseable.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

767e89a

Ticket #31796 : make maxima output parseable.

[sagetrac-git]

Changed commit from a3aa3c9 to 767e89a

[mkoeppe]

comment:21

rm ~/.sage/maxima_commandlist_cache.sobj && ./sage -t --long --random-seed=0 src/sage/symbolic/maxima_wrapper.py gives a different failure here:

File "src/sage/symbolic/maxima_wrapper.py", line 62, in sage.symbolic.maxima_wrapper.MaximaWrapper.__init__
Failed example:
    s.completions('u.elliptic_',globals())
Expected:
    ['u.elliptic_e', 'u.elliptic_ec', 'u.elliptic_eu', 'u.elliptic_f', 'u.elliptic_kc', 'u.elliptic_pi']
Got:
    ['u.elliptic_ec',
     'u.elliptic_eu',
     'u.elliptic_f',
     'u.elliptic_kc',
     'u.elliptic_pi']

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

d7abbe6

MaximaAbstract._commands: Remove whitespace in apropos output before breaking it apart

[sagetrac-git]

Changed commit from 767e89a to d7abbe6

[mkoeppe]

Changed author from Emmanuel Charpentier to Emmanuel Charpentier, Matthias Koeppe

[fchapoton]

comment:25

patchbot reports some trivial failing doctests in it tutorial

[fchapoton]

comment:27

I have fixed the doctests in italian documentation

---

New commits:

d3ec3aa	Merge branch 'u/mkoeppe/legible_maxima_output' in 9.5.b7
9e48365	fix doctest inside it documentation ; wrap some long lines

[fchapoton]

Changed commit from d7abbe6 to 9e48365

[fchapoton]

Changed branch from u/mkoeppe/legible_maxima_output to public/ticket/31796

[mkoeppe]

Changed author from Emmanuel Charpentier, Matthias Koeppe to Emmanuel Charpentier, Matthias Koeppe, Frédéric Chapoton

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

05fa319

some more details

[sagetrac-git]

Changed commit from 9e48365 to 05fa319

[fchapoton]

comment:30

ok, looks good. Patchbot is morally green.

I vote for positive review. Any confirmation from other reviewers or authors ?

[EmmanuelCharpentier]

comment:31

Replying to @fchapoton:

ok, looks good. Patchbot is morally green.

What means "morally" green ???

I vote for positive review. Any confirmation from other reviewers or authors ?

Running ptestlong once more (just in case : this particular patch has already proven not-so-easy to integrate). Stay tuned...

[EmmanuelCharpentier]

comment:32

Replying to @EmmanuelCharpentier:

Replying to @fchapoton:

ok, looks good. Patchbot is morally green.

What means "morally" green ???

I vote for positive review. Any confirmation from other reviewers or authors ?

Running ptestlong once more (just in case : this particular patch has already proven not-so-easy to integrate). Stay tuned...

On Debian testing running on core i7 + 16 GB RAM, running ptestlong on this patch added on top of 9.5.beta7 gives no new failure.

==> positive_review is OK for me. I set it, but feel free to unset if necessary.

[vbraun]

Changed branch from public/ticket/31796 to 05fa319