Coder-Spirit / Jupyter-PHP

A PHP Kernel for Jupyter
MIT License
221 stars 33 forks source link

PHP Double output #17

Closed muotaz closed 7 years ago

muotaz commented 7 years ago

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 commented 7 years ago

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

denisristic commented 7 years ago

same on mac os sierra

muotaz commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

castarco commented 7 years ago

Fixed.

mmorton75 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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:

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.