Skip to content
This repository has been archived by the owner on Dec 27, 2023. It is now read-only.

PHP Double output #17

Closed
muotaz opened this issue Apr 10, 2017 · 10 comments
Closed

PHP Double output #17

muotaz opened this issue Apr 10, 2017 · 10 comments
Assignees

Comments

@muotaz
Copy link

muotaz commented Apr 10, 2017

Hello;

After installing the PHP kernel, jupyter repeats the output returned by the PHP interpreter.
Please refer to the the attached image.

repeated

This issue happens on both an Ubuntu 16.04 and Fedoea 25 machines.

@tumregels
Copy link

the same here, on Ubuntu 16.04 - all output is doubled

@denisristic
Copy link

same on mac os sierra

@muotaz
Copy link
Author

muotaz commented May 2, 2017

Hello;

I was able to remove the duplication after commenting the following lines in ExecuteAction.php which was located at /opt/jupyter-php/pkgs/src/Actions/ :

public function notifyMessage(string $message) 
{
        //$this->broker->send($this->iopubSocket, 'stream', ['name' => 'stdout', 'text' => $message], $this->header);
        $this->broker->send(
            $this->iopubSocket,
            'execute_result',
            ['execution_count' => $this->execCount, 'data' => ['text/plain' => $message], 'metadata' => new \stdClass
            $this->header
        );
    }

And Jupyter recognized as output the lines sent as execute_result rather than stream.
Was the stream needed ?

@castarco
Copy link
Contributor

castarco commented May 3, 2017

Hi all,

@muotaz , @tumregels , @denisristic : excuse me for delaying my responses.

@muotaz : I'll check your patch. To be sincere, I don't remember. I think that this is the way output was generated in first place, and probably it wasn't removed after some PRs solving other more important bugs.

@castarco
Copy link
Contributor

castarco commented May 3, 2017

Confirmed, this fixes the bug without introducing noticeable problems :) .

castarco added a commit that referenced this issue May 3, 2017
Remove duplicated output, following @muotaz suggestion .
@castarco
Copy link
Contributor

castarco commented May 3, 2017

Fixed.

@castarco castarco closed this as completed May 3, 2017
@mmorton75
Copy link

Not sure that this is actually fixed yet. I just built a completely fresh install, in a docker container and we are getting the following output:

screen shot 2017-05-04 at 9 42 26 am

I am also attaching the Dockerfile for the build we used:

Dockerfile.zip

@muotaz
Copy link
Author

muotaz commented May 4, 2017

This is a different issue, you are printing the $text variable, which it does, and then the print function returns a true (1) , if you notice the other cells it is working as expected.

@mmorton75
Copy link

Fair enough - however in the line $text=Hello World"; - it is outputting "Hello World" - that would not be a desired behaviour... and inconsistent with other kernels since you are only setting a variable here.

Also re: the print outputting the text as well as the result of the print statement - this is again, inconsistent with other kernels such as python, where printing does not result in an out[] field - is literally prints it (to stdout?) and supresses in some way the out[]put - only when you do not explicitly print, do you get the out[]put (as in the last example below)

screen shot 2017-05-04 at 11 16 06 am

@castarco
Copy link
Contributor

castarco commented May 4, 2017

Hi @mmorton75 , is true that sending text to stdout should be handled in a different way than expression evaluation.

But, on the other hand, in PHP, an assignment is not only an assignment, but also an expression with a return value, this is the reason behind this weird behavior.

So, I see that you're stating the existence of two problems:

  • the first one is a PHP problem (evaluating assignments as expressions), we could discuss some ways to solve it, but I think that anything that we could do will be brittle.
  • the second one is kernel related (connecting stdout to the "wrong channel").

I also state that there is a third problem: the ugly => "promt" that appears before values. I sent a patch to the "core" of this kernel (PsySH), to solve this, so I expect that by now it will be easy to solve the problem passing an empty string as a "prompt" parameter.

P.D.: I'll create the issue for the second one, meanwhile we can discuss about what to do with the assignments evaluation.

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

No branches or pull requests

5 participants