Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinction between output and result messages #67

Closed
nthiery opened this issue Oct 17, 2017 · 22 comments
Closed

Distinction between output and result messages #67

nthiery opened this issue Oct 17, 2017 · 22 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Oct 17, 2017

Hi,
I just noticed with @takluyver that the xeus-cling output's are not prefixed with [Out ??] prompts:

In [1] 1 + 1
(int) 2

instead of

In [1] 1 + 1
Out[1] (int) 2

This suggests that the output messages are not tagged as "execute_result" as one would expect.

Potentially related: we noticed with @gouarin that nbgrader does not pick error messages when grading xeux-cling notebooks. @takluyver just mentionned that there are two types of error messages which is a regular source of confusion between kernels and frontends. Maybe that's what's happening here, with nbgrader expecting one type, and xeus-cling sending the other.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 17, 2017

In fact, the xeus library (implementing the protocol) supports both streaming cout and cerr to the front-end displaying execute results, which are the two types of outputs involved here

screen shot 2017-10-17 at 9 28 27 pm

But for the xeus-cling kernel, we don't use execute results. The reason is that we capture all the standard and error stream content, including the output, so we avoid the duplication by not using the returned value for an execute_reply message.

If we find a way in the API of the cling interpreter to get that content without it being sent to the standard output, we could have a behavior closer to the one of the python kernel.

@gouarin
Copy link
Member

gouarin commented Oct 17, 2017

I think we can have the same behavior. The execute_reply will happen in xeus-cling only if the last line doesn't have a semicolon.

And cling must be able to give us this information because the behavior is different if you have or you don't have this semicolon. If not, we can add a regexp to know if the last string is followed by a semicolon or not.

Or I missed something ...

@SylvainCorlay
Copy link
Member

I don't think you missed anything!
Ideally, I would prefer not to rely on more regex parsing the code.
Ideally, the behavior should be configurable with the cling API.

@nthiery nthiery changed the title Ouput messages Distinction between output and result messages Oct 17, 2017
@nthiery
Copy link
Contributor Author

nthiery commented Oct 17, 2017

I just realized that this issue is very relevant for teaching. Grasping the distinction between printing and returning is one of the main difficulties for our beginning students. Anything that enhance visually the distinction will help. So a solution will be very welcome!
No rush though, since we won't be able to put a new xeus-cling version in production before the end of the semester.

@nthiery
Copy link
Contributor Author

nthiery commented Oct 17, 2017

Any thoughts about the error message situation?

@SylvainCorlay
Copy link
Member

Any thoughts about the error message situation?

xeus-cling does not do much more about errors than printing into the error stream, which is redirected to the front-end (styled in red in the notebook).

Maybe @takluyver / @ivanov / @ellisonbg would have an opinion about this. We would be happy to follow a better practice.

Regarding the standard output stream versus execute reply, as detailed above, this was a workaround for the redirection of the standard output which also prints the returned value...

@takluyver
Copy link

Is it possible to detect whether execution caused an error or not? If so, it would be useful to send back an indication that the error occurred. There are currently two ways to do that, and we're just working out if it's possible to condense them down to one.

@SylvainCorlay
Copy link
Member

Yes! we catch exception in the kernel layer. How would you like errors to be indicated in the messages (besides outputing things in the error stream).

@takluyver
Copy link

It looks like we're not going to clean up the duplication, so it's best to send both.

@gouarin
Copy link
Member

gouarin commented Oct 18, 2017

I looked at the cling source code. It seems difficult to have the information from cling to know if the end of the last string line is a semicolon or not. It is internal stuff in cling interpreter.

@takluyver could you give two Python examples to illustrate the behavior of the error messages ? one for Error message and one for Error fields.

@SylvainCorlay
Copy link
Member

I looked at the cling source code. It seems difficult to have the information from cling to know if the end of the last string line is a semicolon or not. It is internal stuff in cling interpreter.

This is not really what I would like to do. Instead, I would just like to be able to disable the use of the standard output for execution results by cling, which would be cleaner.

@takluyver
Copy link

could you give two Python examples to illustrate the behavior of the error messages ?

Here's an example in bash_kernel. The call to send_response sends the error message on IOPub, and the return value is the content which will become the execute_reply message.

https://github.com/takluyver/bash_kernel/blob/0966dc102d7549f5c909c93de633a95b2af9f707/bash_kernel/kernel.py#L163-L169

@SylvainCorlay
Copy link
Member

It is not a problem to have exactly the same behavior as in IPython with xeus cling. The only thing issue is that with the current way we interact with cling, (int) 2 would be printed twice, once without the Out[] prompt, then once with it. I would just like to disable the printing of this in the redirected standard output. Ideally, there would be no regex involved...

@gouarin
Copy link
Member

gouarin commented Oct 18, 2017

@SylvainCorlay you're right. So the idea could be to add an output stream in the Value class of cling in order to specify the output when we call Value::dump. This is this function which adds the output of the last line without semicolon. The default value could be cling::outs and in xeus-cling we can change this output stream.

I can open an issue in cling if you are ok with this solution.

@gouarin
Copy link
Member

gouarin commented Oct 18, 2017

@SylvainCorlay
Copy link
Member

This is fixed in master.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Mar 4, 2018

@takluyver @nthiery we released xeus-cling 0.2.1 with this issue fixed.

We also support displaying rich output as per example:

screen shot 2018-03-04 at 10 15 45 pm

Check out the binder!

@nthiery
Copy link
Contributor Author

nthiery commented Mar 4, 2018

Yippee! Thanks!
I am looking forward trying this out, and in particular checking whether this solves our issue with nbgrader.

@SylvainCorlay
Copy link
Member

I am really excited about rich rendering of outputs. Beyond widgets and images, we can display arbittary mime types for which we have a renderer in jupyterlab or the classic notebook...

@SylvainCorlay
Copy link
Member

Quick example with an audio file:

screen shot 2018-03-04 at 10 16 14 pm

@nthiery
Copy link
Contributor Author

nthiery commented Mar 8, 2018

I just tested xeus-cling 0.2.2, and can confirm that nbgrader now works properly.

Steps to reproduce:

Watch out students, gonna grade you :-)

@wolfv
Copy link
Member

wolfv commented Mar 8, 2018

We should also update xtensor-io demo notebooks at some point :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants