Skip to content

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Aug 4, 2016

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?

How should this be tested?

Verify it manually

Screenshots (if appropriate)

2016-08-04_1404

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@zjffdu zjffdu changed the title ZEPPELIN-1287. No need to call print to display output in PythonInter… ZEPPELIN-1287. No need to call print to display output in PythonInterpreter Aug 4, 2016
@zjffdu
Copy link
Contributor Author

zjffdu commented Aug 4, 2016

\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 😃.

import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.InputStreamReader;
import java.io.*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging import to "*" is not recommended. Could you please expand it?

@Leemoonsoo
Copy link
Member

Thanks for the contribution.

Tested and it works great.
Could you rebase and see if it passes CI?

@Leemoonsoo
Copy link
Member

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?

@zjffdu zjffdu closed this Aug 8, 2016
@zjffdu zjffdu reopened this Aug 8, 2016
@Leemoonsoo
Copy link
Member

@zjffdu Thanks for the contribution!

LGTM

@Leemoonsoo
Copy link
Member

Merge to master if there's no more discussion

@minahlee
Copy link
Member

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

@zjffdu
Copy link
Contributor Author

zjffdu commented Aug 10, 2016

+1 for merging this into branch-0.6

1 similar comment
@Leemoonsoo
Copy link
Member

+1 for merging this into branch-0.6

@asfgit asfgit closed this in 3dec4d7 Aug 11, 2016
asfgit pushed a commit that referenced this pull request Aug 12, 2016
…n PythonInterpreter

### What is this PR for?
Implement #1278 to merge branch-0.6

### What type of PR is it?
Bug Fix

### What is the Jira issue?
[ZEPPELIN-1287](https://issues.apache.org/jira/browse/ZEPPELIN-1287)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Mina Lee <minalee@apache.org>

Closes #1320 from minahlee/branch-0.6_ZEPPELIN-1287 and squashes the following commits:

f99f8aa [Mina Lee] return result directly
ac83b14 [Mina Lee] No need to call print to display output in PythonInterpreter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants