This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new acf1cbd  [ZEPPELIN-4562]. Googlevis doesn't work in IRInterpreter
acf1cbd is described below

commit acf1cbde2ca12ab85b740c3e94e543af3954281e
Author: Jeff Zhang <zjf...@apache.org>
AuthorDate: Thu Jan 16 16:08:01 2020 +0800

    [ZEPPELIN-4562]. Googlevis doesn't work in IRInterpreter
    
    ### What is this PR for?
    
    Googlevis doesn't work in IRInterpreter, I fix it by always output html by 
default for IRInterpreter.
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-4562
    
    ### How should this be tested?
    * CI pass, unit test is added
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: Jeff Zhang <zjf...@apache.org>
    
    Closes #3608 from zjffdu/ZEPPELIN-4562 and squashes the following commits:
    
    23f00cda4 [Jeff Zhang] [ZEPPELIN-4562]. Googlevis doesn't work in 
IRInterpreter
---
 .../java/org/apache/zeppelin/r/IRInterpreter.java    |   2 +-
 spark/interpreter/figure/unnamed-chunk-1-1.png       | Bin 0 -> 403630 bytes
 .../spark/PySparkInterpreterMatplotlibTest.java      |   2 +-
 testing/install_external_dependencies.sh             |   4 ++--
 .../zeppelin/interpreter/InterpreterOutput.java      |   6 ++++--
 .../apache/zeppelin/jupyter/JupyterKernelClient.java |   9 +++++++--
 .../zeppelin/jupyter/JupyterKernelInterpreter.java   |   5 +++++
 .../org/apache/zeppelin/jupyter/IRKernelTest.java    |  19 ++++++++++++++++++-
 8 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java 
b/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java
index 949b1b1..f460e84 100644
--- a/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java
+++ b/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java
@@ -133,7 +133,7 @@ public class IRInterpreter extends JupyterKernelInterpreter 
{
             .replace("${timeout}", timeout)
             .replace("${isSparkSupported}", "\"" + isSparkSupported() + "\"")
             .replace("${authSecret}", "\"" + sparkRBackend.socketSecret() + 
"\"");
-    LOGGER.info("Init IRKernel via script:\n" + code);
+    LOGGER.debug("Init IRKernel via script:\n" + code);
     ExecuteResponse response = 
jupyterKernelClient.block_execute(ExecuteRequest.newBuilder()
             .setCode(code).build());
     if (response.getStatus() != ExecuteStatus.SUCCESS) {
diff --git a/spark/interpreter/figure/unnamed-chunk-1-1.png 
b/spark/interpreter/figure/unnamed-chunk-1-1.png
new file mode 100644
index 0000000..e3ca4d1
Binary files /dev/null and b/spark/interpreter/figure/unnamed-chunk-1-1.png 
differ
diff --git 
a/spark/interpreter/src/test/java/org/apache/zeppelin/spark/PySparkInterpreterMatplotlibTest.java
 
b/spark/interpreter/src/test/java/org/apache/zeppelin/spark/PySparkInterpreterMatplotlibTest.java
index 305839a..4f5c020 100644
--- 
a/spark/interpreter/src/test/java/org/apache/zeppelin/spark/PySparkInterpreterMatplotlibTest.java
+++ 
b/spark/interpreter/src/test/java/org/apache/zeppelin/spark/PySparkInterpreterMatplotlibTest.java
@@ -227,7 +227,7 @@ public class PySparkInterpreterMatplotlibTest {
     // again but in a different color.
     ret = pyspark.interpret("plt.plot([1, 2, 3])", context);
     ret2 = pyspark.interpret("plt.show()", context);
-    assertNotSame(ret1.message().get(0).getData(), 
ret2.message().get(0).getData());
+    assertEquals(0, ret2.message().size());
   }
 
   @Test
diff --git a/testing/install_external_dependencies.sh 
b/testing/install_external_dependencies.sh
index 492f37e..b9e1b48 100755
--- a/testing/install_external_dependencies.sh
+++ b/testing/install_external_dependencies.sh
@@ -67,6 +67,6 @@ if [[ "$R" == "true" ]] ; then
   R -e "install.packages('knitr', repos = 'http://cran.us.r-project.org', 
lib='~/R')"  > /dev/null 2>&1
   R -e "install.packages('ggplot2', repos = 'http://cran.us.r-project.org', 
lib='~/R')"  > /dev/null 2>&1
   R -e "install.packages('IRkernel', repos = 'http://cran.us.r-project.org', 
lib='~/R');IRkernel::installspec()" > /dev/null 2>&1
-  R -e "install.packages('shiny', repos = 'http://cran.us.r-project.org', 
lib='~/R');IRkernel::installspec()" > /dev/null 2>&1
-
+  R -e "install.packages('shiny', repos = 'http://cran.us.r-project.org', 
lib='~/R')" > /dev/null 2>&1
+  R -e "install.packages('googleVis', repos = 'http://cran.us.r-project.org', 
lib='~/R')" > /dev/null 2>&1
 fi
diff --git 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
index f85e535..3641222 100644
--- 
a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
+++ 
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
@@ -329,9 +329,11 @@ public class InterpreterOutput extends OutputStream {
     List<InterpreterResultMessage> list = new LinkedList<>();
     synchronized (resultMessageOutputs) {
       for (InterpreterResultMessageOutput out : resultMessageOutputs) {
-        if (out.toInterpreterResultMessage().getType() == 
InterpreterResult.Type.TEXT &&
+        InterpreterResultMessage msg = out.toInterpreterResultMessage();
+        if ((msg.getType() == InterpreterResult.Type.TEXT ||
+                msg.getType() == InterpreterResult.Type.HTML) &&
                 
StringUtils.isBlank(out.toInterpreterResultMessage().getData())) {
-          // skip blank text, because when print table data we usually need to 
print '%text \n'
+          // skip blank text/html, because when print table data we usually 
need to print '%text \n'
           // first to separate it from previous other kind of data. e.g. 
z.show(df)
           continue;
         }
diff --git 
a/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelClient.java
 
b/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelClient.java
index 1f4e200..0e0902b 100644
--- 
a/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelClient.java
+++ 
b/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelClient.java
@@ -22,6 +22,8 @@ import io.grpc.ManagedChannelBuilder;
 import io.grpc.stub.StreamObserver;
 import org.apache.commons.lang.exception.ExceptionUtils;
 import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterResult;
+import org.apache.zeppelin.interpreter.InterpreterResultMessageOutput;
 import org.apache.zeppelin.interpreter.jupyter.proto.JupyterKernelGrpc;
 import org.apache.zeppelin.interpreter.util.InterpreterOutputStream;
 import org.apache.zeppelin.interpreter.jupyter.proto.CancelRequest;
@@ -155,12 +157,15 @@ public class JupyterKernelClient {
                 // the output from jupyter kernel maybe specify format already.
                 
interpreterOutput.write((executeResponse.getOutput()).getBytes());
               } else {
-                // only add %text when the previous output type is not TEXT.
+                // only add %text when the previous output type is not TEXT & 
HTML.
                 // Reason :
                 // 1. if no `%text`, it will be treated as previous output 
type.
                 // 2. Always prepend `%text `, there will be an extra line 
separator,
                 // because `%text ` appends line separator first.
-                if (lastOutputType != OutputType.TEXT) {
+                InterpreterResultMessageOutput curOutput =
+                        
interpreterOutput.getInterpreterOutput().getCurrentOutput();
+                if (curOutput != null && curOutput.getType() != 
InterpreterResult.Type.HTML &&
+                        curOutput.getType() != InterpreterResult.Type.TEXT) {
                   interpreterOutput.write("%text ".getBytes());
                 }
                 
interpreterOutput.write(executeResponse.getOutput().getBytes());
diff --git 
a/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelInterpreter.java
 
b/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelInterpreter.java
index d1dc351..d0e9348 100644
--- 
a/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelInterpreter.java
+++ 
b/zeppelin-jupyter-interpreter/src/main/java/org/apache/zeppelin/jupyter/JupyterKernelInterpreter.java
@@ -245,6 +245,11 @@ public class JupyterKernelInterpreter extends 
AbstractInterpreter {
     interpreterOutput.setInterpreterOutput(context.out);
     jupyterKernelClient.setInterpreterContext(context);
     try {
+      // always use html output for ir kernel. otherwise some R packages 
doesn't work.
+      // e.g. googlevis
+      if (getKernelName().equals("ir")) {
+        context.out.write("%html\n");
+      }
       ExecuteResponse response =
               
jupyterKernelClient.stream_execute(ExecuteRequest.newBuilder().setCode(st).build(),
                       interpreterOutput);
diff --git 
a/zeppelin-jupyter-interpreter/src/test/java/org/apache/zeppelin/jupyter/IRKernelTest.java
 
b/zeppelin-jupyter-interpreter/src/test/java/org/apache/zeppelin/jupyter/IRKernelTest.java
index 002e97b..f3fbffe 100644
--- 
a/zeppelin-jupyter-interpreter/src/test/java/org/apache/zeppelin/jupyter/IRKernelTest.java
+++ 
b/zeppelin-jupyter-interpreter/src/test/java/org/apache/zeppelin/jupyter/IRKernelTest.java
@@ -86,7 +86,7 @@ public class IRKernelTest {
     assertEquals(InterpreterResult.Code.ERROR, result.code());
     resultMessages = context.out.toInterpreterResultMessage();
     assertEquals(1, resultMessages.size());
-    assertEquals(result.toString(), InterpreterResult.Type.TEXT, 
resultMessages.get(0).getType());
+    assertEquals(result.toString(), InterpreterResult.Type.HTML, 
resultMessages.get(0).getType());
     assertTrue(resultMessages.toString(),
             resultMessages.get(0).getData().contains("object 'unknown_var' not 
found"));
 
@@ -99,6 +99,7 @@ public class IRKernelTest {
     assertEquals(resultMessages.toString(),
             InterpreterResult.Type.IMG, resultMessages.get(0).getType());
 
+    // ggplot2
     result = interpreter.interpret("library(ggplot2)\n" +
             "ggplot(diamonds, aes(x=carat, y=price, color=cut)) + 
geom_point()",
             getInterpreterContext());
@@ -107,6 +108,22 @@ public class IRKernelTest {
     assertEquals(1, resultMessages.size());
     assertEquals(resultMessages.toString(),
             InterpreterResult.Type.IMG, resultMessages.get(0).getType());
+
+    // googlevis
+    context = getInterpreterContext();
+    result = interpreter.interpret("library(googleVis)\n" +
+            "df=data.frame(country=c(\"US\", \"GB\", \"BR\"), \n" +
+            "              val1=c(10,13,14), \n" +
+            "              val2=c(23,12,32))\n" +
+            "Bar <- gvisBarChart(df)\n" +
+            "print(Bar, tag = 'chart')", context);
+    assertEquals(InterpreterResult.Code.SUCCESS, result.code());
+    resultMessages = context.out.toInterpreterResultMessage();
+    assertEquals(1, resultMessages.size());
+    assertEquals(resultMessages.toString(),
+            InterpreterResult.Type.HTML, resultMessages.get(0).getType());
+    assertTrue(resultMessages.get(0).getData(),
+            resultMessages.get(0).getData().contains("javascript"));
   }
 
   protected InterpreterContext getInterpreterContext() {

Reply via email to