Comment in python code [[cloud.sagemath.com]]

[Christophe Bal]

Hello.

I think that is not good to have an error with the following code.

----------------------------------------

for i in range(10):

# Here is a basic comment...

    print i, "-->", i**2

----------------------------------------

The error due to the comment is the following one.

Error in lines 1-1 Traceback (most recent call last): File "/mnt/home/pjXx5pJx/.sagemathcloud/sage_server.py", line 633, in execute exec compile(block+'\n', '', 'single') in namespace, locals File "<string>", line 1 for i in range(Integer(10)): ^ SyntaxError: unexpected EOF while parsing

Just add a space before # and everything is ok... Too weird...

This should be fixed.

Best regards.

C.

[Andrew Mathas]

This is not a bug. Rather it is a consequence of the syntax require by python. To quote from the python documentation:

Leading whitespace (spaces and tabs) at the beginning of a logical line is used to compute the indentation level of the line, which in turn is used to determine the grouping of statements.

The reason why you are getting an error here is that in python code, including comment, in for-loops must be indented. The rationale for this is that it makes the code easier to read. I tend to agree because I used to indent my code before I started using python. I even indent in tex files:)

Andrew

[Volker Braun]

No, comments are ignored so their indentation does not matter. So as far as block rules go, you just have an empty line. The trailing whitespace on an empty line has no syntactic meaning.

[Nils Bruin]

On Sunday, November 3, 2013 2:29:20 AM UTC-8, projetmbc wrote:

Hello.

I think that is not good to have an error with the following code.

----------------------------------------

for i in range(10):

# Here is a basic comment...

    print i, "-->", i**2

----------------------------------------

 

I cannot reproduce the error in the notebook nor on the command line. The code executes properly for me.

[William Stein]

On Sun, Nov 3, 2013 at 2:29 AM, Christophe Bal <proj...@gmail.com> wrote:
> Hello.
>
> I think that is not good to have an error with the following code.
>

You're right; this is a bug in Sagemath Cloud's block parser. I've
added it to the issue tracker:

https://github.com/sagemath/cloud/issues/46

William

> ----------------------------------------
> for i in range(10):
> # Here is a basic comment...
> print i, "-->", i**2
> ----------------------------------------
>
> The error due to the comment is the following one.
>
> Error in lines 1-1 Traceback (most recent call last): File
> "/mnt/home/pjXx5pJx/.sagemathcloud/sage_server.py", line 633, in execute
> exec compile(block+'\n', '', 'single') in namespace, locals File "<string>",
> line 1 for i in range(Integer(10)): ^ SyntaxError: unexpected EOF while
> parsing
>
>
> Just add a space before # and everything is ok... Too weird...
> This should be fixed.
>
> Best regards.
> C.
>

> --
> You received this message because you are subscribed to the Google Groups
> "sage-support" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sage-support...@googlegroups.com.
> To post to this group, send email to sage-s...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sage-support.
> For more options, visit https://groups.google.com/groups/opt_out.



--
William Stein
Professor of Mathematics
University of Washington
http://wstein.org

[William Stein]

This is a bug that is my fault in the SMC notebook. I modified how
blocks of code are evaluated, to address a longstanding complaint
people have with sagenb. For example, if you type

2+2
3+5

in sagenb (or IPython notebook), then you see only 8, but in SMC you
see 4 then 8. Also, if you type

for i in range(10):
i
for j in range(5):
j*j

you'll see output from both in SMC, but not in sagenb. In sagenb you
see only the j stuff. In Ipython notebook you see *nothing*.

The relevant function is divide_into_blocks in parsing.py, which I've
attached to this email, in case anybody wants to fix it for me, since
people like Robert Bradshaw and you guys are way better at writing
parsers than I am! :-)

[Christophe Bal]

Thanks for the reactivity. I have no time today to implement this but indeed it is a very simple task to do. If no one gives you something, I can try to adapt your code. Just tell us in this discussion if someone has done the job.

For my part, I could only look at this this french thursday.

Best regards.

Christophe. 

2013/11/3 William Stein <wst...@gmail.com>

[Christophe Bal]

Quickly, what are the functions to change ? What is the feature wanted for parsing.py ? This will help me to try to improve your code. 

2013/11/3 Christophe Bal <proj...@gmail.com>

[Christophe Bal]

Sorry for my question because William wrote :

The relevant function is divide_into_blocks in parsing.py, which I've
attached to this email, in case anybody wants to fix it for me, since
people like Robert Bradshaw and you guys are way better at writing
parsers than I am! :-)

So I will look at that thursday.

2013/11/3 Christophe Bal <proj...@gmail.com>

[Christophe Bal]

Here are two a real questions. ;-) Let me talk please. :-)

Can you give an example of code sent to `divide_into_blocks` ? It seems to be a mixture. 

Another thing. Is `divide_into_blocks` which takes care of docstring ? If it is, I could also try to improve the printing of triple quoting via an optional parameter. 

2013/11/3 Christophe Bal <proj...@gmail.com>

[William Stein]

On Sun, Nov 3, 2013 at 10:41 AM, Christophe Bal <proj...@gmail.com> wrote:
> Here are two a real questions. ;-) Let me talk please. :-)
>
> Can you give an example of code sent to `divide_into_blocks` ? It seems to
> be a mixture.

In any cloud.sagemath project, take a look at

$HOME/.sagemathcloud

and you'll find a couple of (BSD-licensed) Python files:

sage_server.py
sage_salvus.py
parsing.py

I wrote all of these from scratch for SMC. (As it is set up, it can
be painful to debug, unfortunately.)

sage_server.py -- a forking Sage session TCP server. It starts
up, imports the sage library, monkey patches it with functionality
mostly defined in sage_salvus.py, imports a bunch of stuff *in* the
sage library that is normally slow, e.g., graphics, and then waits for
connections. When a connection comes in, it forks off and the fork
handles that session. This is why you can open a new cloud.sagemath
sage worksheet and immediately draw plots, do integrals, etc., without
the significant startup time you might have on the command line or in
sagenb.

sage_salvus.py -- python parts of implementation of the SMC version
of a notebook interface, including a new implementation from scratch
of interact, of 3d graphics, etc.

parsing.py -- when you type code in a cell and press "shift-enter",
it is mostly parsed using functions defined here (and also there is
some relevant code in sage_server.py).


The function divide_into_blocks gets called when you type a bunch of
code into a cell and press shift-enter. Each individual block then
gets exec'd separately, one by one, with execution stopping when you
hit an error. This is why:

2+2
3+3

prints out both 4 and 6.

So to get to the point, open a new worksheet and type this in a cell


import parsing; reload(parsing)

then in another cell, type this

parsing.divide_into_blocks("""

for i in range(10):
# Here is a basic comment...
print i, "-->", i**2

""")

then see this output:

[[0, 0, 'for i in range(10):'], [1, 2, 'print i, "-->", i**2']]

This is wrong, it should be one single block.

Now copy $HOME/.sagemathcloud/parsing.py to somewhere else, say your
home directory, then edit it, and do

sys.path.insert(0,os.environ['HOME']); import parsing; reload(parsing)

and the above (and other) tests.

If you have any trouble getting going with this, just share a project
with me and I can help.

>
> Another thing. Is `divide_into_blocks` which takes care of docstring ? If it
> is, I could also try to improve the printing of triple quoting via an
> optional parameter.

I don't understand the question yet.

[Volker Braun]

Manually parsing python code is spectacularly ugly and, as we saw, error prone. The right way to do it is to use Python's own parser and then either take apart the ast or use a NodeTransformer to insert the desired print statements at the ends of if-bodies.

[William Stein]

Unfortunately, the input isn't Python code, it's sage worksheet code,
because of both the preparser and the % cell and line decorators.

Fortunately, I think there's no insertion of actual print statements.
Python has something called sys.displayhook, which gets called
automatically when exec'ing.

[Volker Braun]

The displayhook just does print repr(x) and print a little bit nicer in Sage. It would be better to call the displayhook than printing directly, but thats not the issue here.

When I said that you need to walk the ast then of course you have to first preparse, then compile the ast.

[Nils Bruin]

On Sunday, November 3, 2013 11:43:35 AM UTC-8, William wrote:

Unfortunately, the input isn't Python code, it's sage worksheet code,
because of both the preparser and the % cell and line decorators.

But once you've stripped and processed those bits, what you're left with is something that gets fed to Python.
A python block is a sequence of statements, some of them bare expressions.
The old notebook (sort of) prints the result of the last statement if it's an expression, but even then really only if it fits on a line. Compare
{{{
[1,2,3]
///
[1,2,3]
}}}
(works as expected) and
{{{
[1,2,
3]
///
}}}
(nothing printed, because expression doesn't start on last line)

What we would LIKE, and what you seem to be trying in cloud from what I understand, is to print/display the results of all statements in the block that are expressions. Instead of trying to chop up the block into statements yourself, you could let Python's parser do that, look which statements are bare expressions, and wrap those with the appropriate code to send the result somewhere:

def wrap_with_print(c):
    R=ast.parse("sys.displayhook(None)").body[0]
    R.value.args[0]=c.value
    return R

def execute_block_with_display(code_string):
    M=ast.parse(code_string)
    M.body=[wrap_with_print(s) if isinstance(s,ast.Expr) else s for s in M.body]
    c=compile(M,"<string>","exec")
    eval(c,globals())

S="""
a=1
("value of a=",a)
a+=1
("value of a=",a)
"""
execute_block_with_display(S)

this should be much more robust than trying to roll your own partial implementation of a python parser. Note that you only have to do modifications on top level, so there is not much ast "walking" involved.

[Nils Bruin]

On Sunday, November 3, 2013 12:42:04 PM UTC-8, Nils Bruin wrote:

Note that you only have to do modifications on top level, so there is not much ast "walking" involved.

That is of course only true if you want that

for i in range(1000):
    i

doesn't produce output (which I think is reasonable, but not what python's command line does. I guess compile(...,...,"single") does that. So perhaps you'd want something along the lines of:

S="""
for i in range(10):
    i+1
for j in range(10):
    j-1
"""

def execute_block_interactively(code_string):
    M=ast.parse(code_string)
    c=compile(ast.Interactive(M.body),"<string>","single")
    eval(c)

execute_block_interactively(S)

[Christophe Bal]

Hello.

The use of AST is a pretty way BUT you must not use eval or exec because of real security issues. It's easy to find explanations about that on the web. 

Christophe 

2013/11/3 Nils Bruin <nbr...@sfu.ca>

--

[Nils Bruin]

On Sunday, November 3, 2013 11:19:45 PM UTC-8, projetmbc wrote:

The use of AST is a pretty way BUT you must not use eval or exec because of real security issues. It's easy to find explanations about that on the web. 

If you read these explanations, you'll see that by the same logic, you shouldn't run a notebook server because of real security issues (and if you don't understand that, then you should indeed not run a notebook server accessible to other people). The code that is input into a notebook is already run via something equivalent to exec (try and think of another way of letting sage do what it does). The code proposed is not less secure than what we're already doing in the notebook.

[Christophe Bal]

Indeed there are small security problems and big ones. The use of eval or exec can cause real big problems. I'll try to show that in private. Not here... I'm not sure to do such a hack but if the actual version uses exec or eval, it would be possible.

My remark is just to help and not to criticize freely. Sorry for my English because this is not my natural language.

Best regards.
Christophe

--

[William Stein]

On Mon, Nov 4, 2013 at 6:04 AM, Christophe Bal <proj...@gmail.com> wrote:
> Indeed there are small security problems and big ones. The use of eval or
> exec can cause real big problems. I'll try to show that in private. Not
> here... I'm not sure to do such a hack but if the actual version uses exec
> or eval, it would be possible.
>
> My remark is just to help and not to criticize freely. Sorry for my English
> because this is not my natural language.
>

Fortunately, in the context of https://cloud.sagemath.com there are
absolutely no security issues associated with using exec, eval, etc.
This is because all relevant Python code is run in an isolated virtual
machine, in which the user is explicitly given -- by the security
model -- full shell access (in that VM). That's made clear in this
case, since there is literally a terminal in cloud.sagemath.

There are other contexts where exec/eval must be avoided, but this
isn't one of them, fortunately.

William

[Christophe Bal]

Thanks. Where all of this I'd implemented ?

[William Stein]

On Mon, Nov 4, 2013 at 7:28 AM, Christophe Bal <proj...@gmail.com> wrote:
> Thanks. Where all of this I'd implemented ?

Don't worry about it further -- I'll fix the parsing bug that you
found sometime this week. If you find any further bugs anywhere in
cloud.sagemath.com, please don't hesitate to report them! Thanks
again,

William