Backslash line continuation broken when defining a callable symbolic expression

[egourgoulhon]

As reported in https://groups.google.com/g/sage-devel/c/XD1VtG0TOEk/m/Lo6L8YBGCgAJ, we have currently (Sage 9.2 and 9.3.beta1):

sage: f(x) = x \ 
....:        + 1                                                                                    
  File "<ipython-input-1-a5c1e14e19da>", line 1
    __tmp__=var("x"); f = symbolic_expression(x  * BackslashOperator() * ).function(x)
                                                                         ^
SyntaxError: invalid syntax

There was no such issue with Sage 9.1 and lower.

CC: @jcamp0x2a @slel

Component: symbolics

Keywords: callable symbolic expression, backslash, preparser

Author: Joshua Campbell

Branch/Commit: 64c5757

Reviewer: Steven Trogdon, Eric Gourgoulhon

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

[egourgoulhon]

comment:1

The issue is already there in Sage 9.2.beta14.

[egourgoulhon]

comment:2

FWIW, in Sage 9.3.beta1, we have

sage: preparse(r"f(x) = x \ + 1")                                                                  
'__tmp__=var("x"); f = symbolic_expression(x  * BackslashOperator() * + Integer(1)).function(x)'

which is correct, while in the reported issue the second line is entirely missing after BackslahOperator() *, which triggers the syntax error.

[egourgoulhon]

comment:3

The bug could arise from the upgrade to ipython 7 performed in #28197 (merged in Sage 9.2.beta8) or from the follow-up ticket #30417 (merged in Sage 9.2.beta11).

[strogdon]

comment:4

I may be off base, but the following may be important

sage: from sage.repl.interpreter import get_test_shell                                                                                                                                               
sage: shell = get_test_shell()                                                                                                                                                                       
sage: shell.run_cell('f(x) = x \ + 1')                                                                                                                                                               
<input>:1: DeprecationWarning: invalid escape sequence \ 
<>:1: DeprecationWarning: invalid escape sequence \ 
<ipython-input-19-e03ae3fa6f44>:1: DeprecationWarning: invalid escape sequence \ 
  shell.run_cell('f(x) = x \ + 1')
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-19-81598207d80e> in <module>
----> 1 __tmp__=var("x"); f = symbolic_expression(x  * BackslashOperator() * + Integer(1)).function(x)

/local/sage-git/sage/local/lib/python3.8/site-packages/sage/misc/misc.py in __mul__(self, right)
    829             (0.0, 0.5, 1.0, 1.5, 2.0)
    830         """
--> 831         return self.left._backslash_(right)
    832 
    833 

/local/sage-git/sage/local/lib/python3.8/site-packages/sage/structure/element.pyx in sage.structure.element.Element.__getattr__ (build/cythonized/sage/structure/element.c:4701)()
    491             AttributeError: 'LeftZeroSemigroup_with_category.element_class' object has no attribute 'blah_blah'
    492         """
--> 493         return self.getattr_from_category(name)
    494 
    495     cdef getattr_from_category(self, name):

/local/sage-git/sage/local/lib/python3.8/site-packages/sage/structure/element.pyx in sage.structure.element.Element.getattr_from_category (build/cythonized/sage/structure/element.c:4813)()
    504         else:
    505             cls = P._abstract_element_class
--> 506         return getattr_from_other_class(self, cls, name)
    507 
    508     def __dir__(self):

/local/sage-git/sage/local/lib/python3.8/site-packages/sage/cpython/getattr.pyx in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:2620)()
    370         dummy_error_message.cls = type(self)
    371         dummy_error_message.name = name
--> 372         raise AttributeError(dummy_error_message)
    373     attribute = <object>attr
    374     # Check for a descriptor (__get__ in Python)

AttributeError: 'sage.symbolic.expression.Expression' object has no attribute '_backslash_'
<ExecutionResult object at 7f213d0cd130, execution_count=None error_before_exec=None error_in_exec='sage.symbolic.expression.Expression' object has no attribute '_backslash_' info=<ExecutionInfo object at 7f213d09fc40, raw_cell="f(x) = x \ + 1" store_history=False silent=False shell_futures=True> result=None>

[strogdon]

comment:5

I get the same if f(x) = x \ + 1 is typed at the sage prompt. So probably not the correct thing to try.

[egourgoulhon]

comment:6

Replying to @strogdon:

I get the same if f(x) = x \ + 1 is typed at the sage prompt. So probably not the correct thing to try.

Yes probably; moreover shell.run_cell(r'f(x) = x \ + 1') yields the same AttributeError in Sage 9.1, where everything is fine with backslash line continuation.

[strogdon]

comment:7

Some data points. I think sage/repl/preparser.py and perhaps sage/repl/interpreter.py need significant changes. Adding print statements as

diff --git a/src/sage/repl/interpreter.py b/src/sage/repl/interpreter.py
index b70345f940..206041e4a1 100644
--- a/src/sage/repl/interpreter.py
+++ b/src/sage/repl/interpreter.py
@@ -444,6 +444,7 @@ def SagePreparseTransformer(lines):
         sage: # instead of ["'''", 'abc-Integer(1)-Integer(2)', "'''"]
 
     """
+    print('we are now in sage.repl.interpreter.SagePreparseTransformer')
     lines_out = []
     reset = True
     for line in lines:
diff --git a/src/sage/repl/preparse.py b/src/sage/repl/preparse.py
index 3619fd9e54..2073caa855 100644
--- a/src/sage/repl/preparse.py
+++ b/src/sage/repl/preparse.py
@@ -1745,6 +1745,8 @@ def preparse(line, reset=True, do_time=False, ignore_prompts=False,
         quote_state = None
 
     L = line.lstrip()
+    print('result of L = line.lstrip() in sage.repl.preparse')
+    print(L)
     if len(L) > 0 and L[0] in ['#', '!']:
         return line

I see:

sage: f(x) = x \ 
....:        + 1                                                                
we are now in sage.repl.interpreter.SagePreparseTransformer
result of L = line.lstrip() in sage.repl.preparse
f(x) = x \

result of L = line.lstrip() in sage.repl.preparse
+ 1

we are now in sage.repl.interpreter.SagePreparseTransformer
result of L = line.lstrip() in sage.repl.preparse
f(x) = x \

result of L = line.lstrip() in sage.repl.preparse
+ 1

  File "<ipython-input-1-a5c1e14e19da>", line 1
    __tmp__=var("x"); f = symbolic_expression(x  * BackslashOperator() * ).function(x)
                                                                         ^
SyntaxError: invalid syntax

Note that preparse is called from interpreter and it is called twice! I don't believe it should be called twice. Furthermore preparse treats the input as two separate lines instead of one line separated by the \ continuation character. This results in \ being interpreted as the * BackslashOperator() *. I'm unable to test for a properly-functioning Sage.

[jcamp0x2a]

comment:9

I think this may have been introduced by the transition to IPython 7 and how its new input transformer API deals with lines. For example, commenting out the backslash substitution altogether in preparse.py still yields:

sage: f(x) = x \
....:        + 1
  File "<ipython-input-1-bc21d2198ee4>", line 1
    __tmp__=var("x"); f = symbolic_expression(x \).function(x)
                                                              ^
SyntaxError: unexpected character after line continuation character

In the old IPython input transformer docs, it appears there were three types of transformer: physical line, logical line, and python line. In the same docs for IPython 7, there now only appears to be two: cleanup and post.

Judging by the change to ipython_extension.py for #28197, it looks like the SagePromptTransformer was moved from "physical line" to "cleanup" and SagePreparseTransformer from "python line" to "post".

I'm wondering if that "post" is not quite the same as the old "python line" however and functions more like the old "logical line". That could explain why #30417 that I fixed arose in the first place.

I have a couple ideas for how to patch this in the short term, but neither is likely to be particularly elegant. I'll play around with them and see if I can get something pushed in the next couple days.

I agree with @strogdon that some significant changes/cleanup are needed in the preparser and interpreter in order to fix this properly. I know there was talk in #28974 about building a proper grammar and parser for Sage to remove the need for all these regex kludges. Would be a large undertaking though.

[jcamp0x2a]

Commit: 68afcdb

[jcamp0x2a]

Branch: u/gh-jcamp0x2a/30928-backslash-continuation

[jcamp0x2a]

Author: Joshua Campbell

[jcamp0x2a]

comment:10

I've pushed a potential fix.

• preparse removes backslash+newline combinations, putting such constructions onto the same line. This happens after strip_string_literals so we don't have to worry about backslash continuations inside strings, and it happens before preparse_calculus so that it can surround the entire thing with a symbolic_expression call.

• For the IPython use case, I've just squashed the list of lines into a single string and passed that to preparse instead of going line-by-line. The prompts should've already been removed by the SagePromptTransformer so I think that's a safe operation. Also, since IPython has already handled the % magic commands immediately prior to our SagePreparseTransformer, I removed the explicit check for them.

• I've done something similar for the preparse_file use case with the complication of supporting load and attach statements. Basically, just calls preparse on batches between such statements and between them and the beginning/end of file.

As predicted: not the most elegant fix, and there's a fair chance I broke another edge case somewhere in the process. I'm not sure how else to fix it without tearing up a bunch of the existing preparser code. Input most welcome! :)

---

New commits:

68afcdb

Fix backslash continuation for symbolic expressions

[strogdon]

comment:11

Initial testing indicates that line continuation is now working. However, regardless the input after the sage: prompt, that input is parsed twice. Perhaps this is a separate issue or maybe not an issue?

Example:

sage: f(x) = x \ 
....:        + 1 \ 
....:        + x^2                                                                                                                                                                                   
we are now in sage.repl.interpreter.SagePreparseTransformer
result of L = line.lstrip() in sage.repl.preparse
f(x) = x \
       + 1 \
       + x^2

we are now in sage.repl.interpreter.SagePreparseTransformer
result of L = line.lstrip() in sage.repl.preparse
f(x) = x \
       + 1 \
       + x^2

sage: f(x)                                                                                                                                                                                           
we are now in sage.repl.interpreter.SagePreparseTransformer
result of L = line.lstrip() in sage.repl.preparse
f(x)

we are now in sage.repl.interpreter.SagePreparseTransformer
result of L = line.lstrip() in sage.repl.preparse
f(x)

x^2 + x + 1

[jcamp0x2a]

comment:12

Hmm... perhaps this is an IPython quirk? For example, running an IPython installed completely independently of Sage:

$ ~/.local/bin/ipython3
Python 3.6.9 (default, Oct  8 2020, 12:12:24)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.16.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: def transform(lines):
   ...:     print(lines)
   ...:     return lines
   ...:

In [2]: get_ipython().input_transformers_post.append(transform)

In [3]: 2 + 2
['2 + 2\n']
['2 + 2\n']
Out[3]: 4

[strogdon]

comment:13

Same here with Python 3.8.6. Bug or feature?

[jcamp0x2a]

comment:14

Replying to @strogdon:

Same here with Python 3.8.6. Bug or feature?

Looks like intended behavior:

• Issue: 11714 "Arrow printed twice for autocall"

• Pull request: 12456 "Allow to mark transformers as having side effects"

The new has_side_effects attribute would only work in IPython >= 7.17.0, and Sage only requires 7.13.0. Besides, the only side effects here should be our debugging print outs, so I vote to just let IPython do its thing.

I'll add a bit about that attribute in SagePreparseTransformer's docstring for those that stumble across this in the future.

[sagetrac-git]

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

e45c717

Explain duplicated print outs in SagePreparseTransformer

[sagetrac-git]

Changed commit from 68afcdb to e45c717

[strogdon]

comment:16

Replying to @jcamp0x2a:

Replying to @strogdon:

Same here with Python 3.8.6. Bug or feature?

Looks like intended behavior:

• Issue: 11714 "Arrow printed twice for autocall"

• Pull request: 12456 "Allow to mark transformers as having side effects"

Good spotting this. I tried to search the ipython git repo bugs for this without success. I never considered that it was related to a closed issue. I have ipython-7.18.1 here on my Gentoo laptop. Your above ipython example, after importing inputtransformer2 from IPython.core and then setting inputtransformer2.has_side_effects = True, does work

In [5]: 2 + 2
['2 + 2\n']
Out[5]: 4

I'm satisfied with this, but will wait to see if @egourgoulhon has any comments.

[strogdon]

comment:17

Replying to @strogdon:

I'm satisfied with this, but will wait to see if @egourgoulhon has any comments.

Or if anyone else has comments.

[egourgoulhon]

comment:18

Thanks for the fix!

I made a few tests and everything is OK. So I am +1 for a positive review, once the pyflake error reported by the patchbot error has been fixed.

[egourgoulhon]

comment:19

Btw, while making tests for this ticket branch, I found another error:

sage: f(x) = (x +  
....:         1)                                                                                    
  File "<ipython-input-10-ae23cb631161>", line 1
    __tmp__=var("x"); f = symbolic_expression((x + ).function(x)
                                                   ^
SyntaxError: invalid syntax

But this does not pertain to the current ticket: it is already here in Sage 9.1.

[jcamp0x2a]

comment:20

Replying to @egourgoulhon:

Thanks for the fix!

I made a few tests and everything is OK. So I am +1 for a positive review, once the pyflake error reported by the patchbot error has been fixed.

I think this is the same false positive that was identified in #30417 comment:8. Trying to ignore it with # noqa didn't work, so I will use an assertion to silence pyflakes for good.

[jcamp0x2a]

comment:21

Replying to @egourgoulhon:

Btw, while making tests for this ticket branch, I found another error:

sage: f(x) = (x +  
....:         1)                                                                                    
  File "<ipython-input-10-ae23cb631161>", line 1
    __tmp__=var("x"); f = symbolic_expression((x + ).function(x)
                                                   ^
SyntaxError: invalid syntax

But this does not pertain to the current ticket: it is already here in Sage 9.1.

The code is still fresh in my mind, and this seems like a trivial fix, so I'm going to try to knock it out here. If it's not as trivial as I thought, though, I'll spin off a new ticket and put this back into review.

[egourgoulhon]

comment:22

Replying to @jcamp0x2a:

The code is still fresh in my mind, and this seems like a trivial fix, so I'm going to try to knock it out here. If it's not as trivial as I thought, though, I'll spin off a new ticket and put this back into review.

OK very good!

[sagetrac-git]

Changed commit from e45c717 to 64c5757

[sagetrac-git]

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

09b499c	Silence false positive from pyflakes
64c5757	preparse_calculus: allow vars to span mulitple lines

[jcamp0x2a]

comment:24

Not trivial. Forgot about nested parentheses/braces/brackets. A regular expression is really ill-suited to handling that, so preparse_calculus is going to need some work. Can use backslash continuations as a workaround for now.

On the bright side, I was able to quickly add support for breaking the parameter list onto multiple lines like so:

f(a,
  b,
  c,
  d) = a + b*2 + c*3 + d*4

...and we'll see what pyflakes has to say now. :)

[jcamp0x2a]

comment:25

Replying to @jcamp0x2a:

Not trivial. Forgot about nested parentheses/braces/brackets. A regular expression is really ill-suited to handling that, so preparse_calculus is going to need some work. Can use backslash continuations as a workaround for now.

Created #30953 to track this.

[strogdon]

comment:26

Not sure what the source of the Darwin failures is. Every thing is fine here with beta2. I believe I've seen other false negative failures on Darwin.

[jcamp0x2a]

comment:27

Replying to @strogdon:

Not sure what the source of the Darwin failures is. Every thing is fine here with beta2. I believe I've seen other false negative failures on Darwin.

Yea, doesn't look like a very happy patchbot. :) Looking through it's history, I don't see a single green TestsPassed result, so I don't think it's due to anything introduced here.

[strogdon]

Reviewer: Steven Trogdon

[strogdon]

comment:28

Eric please add your name to reviewers if you agree.

[egourgoulhon]

comment:29

Thank you very much for having fixed this!

[egourgoulhon]

Changed reviewer from Steven Trogdon to Steven Trogdon, Eric Gourgoulhon

[jcamp0x2a]

comment:30

Replying to @egourgoulhon:

Thank you very much for having fixed this!

You are most welcome!

I'm going to spend some time brainstorming ways to make the preparser more robust, potentially including building that proper grammar/parser I mentioned earlier. Might have to be a year-long goal for 2021, though. :)

[vbraun]

Changed branch from u/gh-jcamp0x2a/30928-backslash-continuation to 64c5757