You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zjffdu <gi...@git.apache.org> on 2016/08/04 06:10:32 UTC

[GitHub] zeppelin pull request #1278: ZEPPELIN-1287. No need to call print to display...

GitHub user zjffdu opened a pull request:

    https://github.com/apache/zeppelin/pull/1278

    ZEPPELIN-1287. No need to call print to display output in PythonInter\u2026

    ### What is this PR for?
    It is not necessary to call print to display output in PythonInterpreter. 2 main changes:
    * the root cause is the displayhook in bootstrap.py
    * also did some code refactoring on PythonInterpreter
    
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-1287
    
    ### How should this be tested?
    Verify it manually
    
    ### Screenshots (if appropriate)
    ![2016-08-04_1404](https://cloud.githubusercontent.com/assets/164491/17392006/090279d2-5a4d-11e6-840b-4cddb595a42e.png)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    \u2026preter

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zjffdu/zeppelin ZEPPELIN-1287

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1278.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1278
    
----
commit f8e52b47b01f7021abe11d6dec65073a60129061
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-08-04T06:05:22Z

    ZEPPELIN-1287. No need to call print to display output in PythonInterpreter

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin pull request #1278: ZEPPELIN-1287. No need to call print to display...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/1278


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    Merge to master if there's no more discussion


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    \\cc @bzz @Leemoonsoo  ready for review,  not sure whether the test fail is relevant. CI is very unstable recently, we might need to fix it asap :smiley:. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    Looks like test is not passing
    
    ```
    -------------------------------------------------------
     T E S T S
    -------------------------------------------------------
    Running org.apache.zeppelin.python.PythonInterpreterTest
    SLF4J: Class path contains multiple SLF4J bindings.
    SLF4J: Found binding in [jar:file:/home/travis/build/apache/zeppelin/zeppelin-interpreter/target/zeppelin-interpreter-0.7.0-SNAPSHOT.jar!/org/slf4j/impl/StaticLoggerBinder.class]
    SLF4J: Found binding in [jar:file:/home/travis/build/apache/zeppelin/zeppelin-interpreter/target/lib/slf4j-log4j12-1.7.10.jar!/org/slf4j/impl/StaticLoggerBinder.class]
    SLF4J: Found binding in [jar:file:/home/travis/.m2/repository/org/slf4j/slf4j-log4j12/1.7.10/slf4j-log4j12-1.7.10.jar!/org/slf4j/impl/StaticLoggerBinder.class]
    SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
    SLF4J: Actual binding is of type [org.slf4j.impl.Log4jLoggerFactory]
    log4j:WARN No appenders could be found for logger (org.apache.zeppelin.interpreter.Interpreter).
    log4j:WARN Please initialize the log4j system properly.
    log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
    Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.629 sec <<< FAILURE! - in org.apache.zeppelin.python.PythonInterpreterTest
    testInterpret(org.apache.zeppelin.python.PythonInterpreterTest)  Time elapsed: 0.015 sec  <<< FAILURE!
    org.junit.ComparisonFailure: expected:<%text []print a> but was:<%text [>>>]print a>
    	at org.junit.Assert.assertEquals(Assert.java:115)
    	at org.junit.Assert.assertEquals(Assert.java:144)
    	at org.apache.zeppelin.python.PythonInterpreterTest.testInterpret(PythonInterpreterTest.java:161)
    
    
    Results :
    
    Failed tests: 
      PythonInterpreterTest.testInterpret:161 expected:<%text []print a> but was:<%text [>>>]print a>
    
    Tests run: 5, Failures: 1, Errors: 0, Skipped: 0
    ```
    @zjffdu Could you take a look?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    Thanks for the contribution.
    
    Tested and it works great. 
    Could you rebase and see if it passes CI?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    +1 for merging this into branch-0.6 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    +1 for merging this into branch-0.6


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    @zjffdu Thanks for the contribution!
    
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin pull request #1278: ZEPPELIN-1287. No need to call print to display...

Posted by zjffdu <gi...@git.apache.org>.
GitHub user zjffdu reopened a pull request:

    https://github.com/apache/zeppelin/pull/1278

    ZEPPELIN-1287. No need to call print to display output in PythonInterpreter

    ### What is this PR for?
    It is not necessary to call print to display output in PythonInterpreter. 2 main changes:
    * the root cause is the displayhook in bootstrap.py
    * also did some code refactoring on PythonInterpreter
    
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-1287
    
    ### How should this be tested?
    Verify it manually
    
    ### Screenshots (if appropriate)
    ![2016-08-04_1404](https://cloud.githubusercontent.com/assets/164491/17392006/090279d2-5a4d-11e6-840b-4cddb595a42e.png)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zjffdu/zeppelin ZEPPELIN-1287

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1278.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1278
    
----
commit 0eade713f1609c77518e19bec2c8750bc3447367
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-08-04T06:05:22Z

    ZEPPELIN-1287. No need to call print to display output in PythonInterpreter

commit 3e9f1699ee77b1a3b38ee5fef168d82d470831af
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-08-04T12:19:41Z

    address comments

commit b48b56fbe68dfd3e3a20a71babac72ada52c02d8
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-08-08T00:40:46Z

    fix unit test fail

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #1278: ZEPPELIN-1287. No need to call print to display output...

Posted by minahlee <gi...@git.apache.org>.
Github user minahlee commented on the issue:

    https://github.com/apache/zeppelin/pull/1278
  
    Can we merge this into branch-0.6 too? FYI #1232 is merged to branch-0.6. It will be nice to provide same user experience both in pyspark and python interpreter


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin pull request #1278: ZEPPELIN-1287. No need to call print to display...

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu closed the pull request at:

    https://github.com/apache/zeppelin/pull/1278


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---