You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2017/04/18 09:52:34 UTC

zeppelin git commit: ZEPPELIN-2390. Improve returnType for z.checkbox

Repository: zeppelin
Updated Branches:
  refs/heads/master 33663941c -> a7ffc1291


ZEPPELIN-2390. Improve returnType for z.checkbox

### What is this PR for?
Currently it is not convenient to access the individual item of the return value of z.checkbox, I would propose to return `Seq` for `SparkInterpreter` and `list` for `PySparkInterpreter`.  This might cause some incompatibility, but I think it is acceptable considering the benefits.  Besides that, before this PR, all the items of checkbox would be checked by default in `PySparkInterpreter` which is inconsistent with `SparkInterpreter`, so I change it to nothing is selected by default in this PR.

### What type of PR is it?
[Improvement]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-2390

### How should this be tested?
Unit test is added

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

Author: Jeff Zhang <zj...@apache.org>

Closes #2241 from zjffdu/ZEPPELIN-2390 and squashes the following commits:

75e3afc [Jeff Zhang] ZEPPELIN-2390. Improve returnType for z.checkbox


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/a7ffc129
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/a7ffc129
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/a7ffc129

Branch: refs/heads/master
Commit: a7ffc1291885cc555ed84bd71f455ebcb2e9dd62
Parents: 3366394
Author: Jeff Zhang <zj...@apache.org>
Authored: Tue Apr 11 18:09:35 2017 +0800
Committer: Jeff Zhang <zj...@apache.org>
Committed: Tue Apr 18 17:52:27 2017 +0800

----------------------------------------------------------------------
 .../apache/zeppelin/spark/ZeppelinContext.java  |  9 ++--
 .../main/resources/python/zeppelin_pyspark.py   | 10 ++--
 .../java/org/apache/zeppelin/display/GUI.java   |  4 +-
 .../zeppelin/rest/ZeppelinSparkClusterTest.java | 54 +++++++++++++++++---
 4 files changed, 61 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a7ffc129/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
----------------------------------------------------------------------
diff --git a/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java b/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
index 6e96d9d..e187915 100644
--- a/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
+++ b/spark/src/main/java/org/apache/zeppelin/spark/ZeppelinContext.java
@@ -136,7 +136,7 @@ public class ZeppelinContext {
   }
 
   @ZeppelinApi
-  public scala.collection.Iterable<Object> checkbox(String name,
+  public scala.collection.Seq<Object> checkbox(String name,
       scala.collection.Iterable<Tuple2<Object, String>> options) {
     List<Object> allChecked = new LinkedList<>();
     for (Tuple2<Object, String> option : asJavaIterable(options)) {
@@ -146,11 +146,12 @@ public class ZeppelinContext {
   }
 
   @ZeppelinApi
-  public scala.collection.Iterable<Object> checkbox(String name,
+  public scala.collection.Seq<Object> checkbox(String name,
       scala.collection.Iterable<Object> defaultChecked,
       scala.collection.Iterable<Tuple2<Object, String>> options) {
-    return collectionAsScalaIterable(gui.checkbox(name, asJavaCollection(defaultChecked),
-      tuplesToParamOptions(options)));
+    return scala.collection.JavaConversions.asScalaBuffer(
+        gui.checkbox(name, asJavaCollection(defaultChecked),
+            tuplesToParamOptions(options))).toSeq();
   }
 
   private ParamOption[] tuplesToParamOptions(

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a7ffc129/spark/src/main/resources/python/zeppelin_pyspark.py
----------------------------------------------------------------------
diff --git a/spark/src/main/resources/python/zeppelin_pyspark.py b/spark/src/main/resources/python/zeppelin_pyspark.py
index 5029d59..da4d794 100644
--- a/spark/src/main/resources/python/zeppelin_pyspark.py
+++ b/spark/src/main/resources/python/zeppelin_pyspark.py
@@ -97,13 +97,15 @@ class PyZeppelinContext(dict):
 
   def checkbox(self, name, options, defaultChecked=None):
     if defaultChecked is None:
-      defaultChecked = list(map(lambda items: items[0], options))
+      defaultChecked = []
     optionTuples = list(map(lambda items: self.__tupleToScalaTuple2(items), options))
     optionIterables = gateway.jvm.scala.collection.JavaConversions.collectionAsScalaIterable(optionTuples)
     defaultCheckedIterables = gateway.jvm.scala.collection.JavaConversions.collectionAsScalaIterable(defaultChecked)
-
-    checkedIterables = self.z.checkbox(name, defaultCheckedIterables, optionIterables)
-    return gateway.jvm.scala.collection.JavaConversions.asJavaCollection(checkedIterables)
+    checkedItems = gateway.jvm.scala.collection.JavaConversions.seqAsJavaList(self.z.checkbox(name, defaultCheckedIterables, optionIterables))
+    result = []
+    for checkedItem in checkedItems:
+      result.append(checkedItem)
+    return result;
 
   def registerHook(self, event, cmd, replName=None):
     if replName is None:

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a7ffc129/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/GUI.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/GUI.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/GUI.java
index 30a8ba7..40ce8ca 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/GUI.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/display/GUI.java
@@ -75,14 +75,14 @@ public class GUI implements Serializable {
     return value;
   }
 
-  public Collection<Object> checkbox(String id, Collection<Object> defaultChecked,
+  public List<Object> checkbox(String id, Collection<Object> defaultChecked,
                                      ParamOption[] options) {
     Collection<Object> checked = (Collection<Object>) params.get(id);
     if (checked == null) {
       checked = defaultChecked;
     }
     forms.put(id, new Input(id, defaultChecked, "checkbox", options));
-    Collection<Object> filtered = new LinkedList<>();
+    List<Object> filtered = new LinkedList<>();
     for (Object o : checked) {
       if (isValidOption(o, options)) {
         filtered.add(o);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a7ffc129/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
index 9bcdbfa..eb8186e 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
@@ -483,18 +483,19 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi {
     }
 
     @Test
-    public void testZeppelinContextDynamicForms() throws IOException {
+    public void testSparkZeppelinContextDynamicForms() throws IOException {
         Note note = ZeppelinServer.notebook.createNote(anonymous);
         Paragraph p = note.addParagraph(AuthenticationInfo.ANONYMOUS);
         note.setName("note");
         Map config = p.getConfig();
         config.put("enabled", true);
         p.setConfig(config);
-        String code = "%spark.spark z.input(\"my_input\", \"default_name\")\n" +
-            "z.select(\"my_select\", \"select_2\"," +
-            "Seq((\"1\", \"select_1\"), (\"2\", \"select_2\")))\n" +
-            "z.checkbox(\"my_checkbox\", Seq(\"check_1\"), " +
-            "Seq((\"1\", \"check_1\"), (\"2\", \"check_2\")))";
+        String code = "%spark.spark println(z.input(\"my_input\", \"default_name\"))\n" +
+            "println(z.select(\"my_select\", \"1\"," +
+            "Seq((\"1\", \"select_1\"), (\"2\", \"select_2\"))))\n" +
+            "val items=z.checkbox(\"my_checkbox\", Seq(\"2\"), " +
+            "Seq((\"1\", \"check_1\"), (\"2\", \"check_2\")))\n" +
+            "println(items(0))";
         p.setText(code);
         p.setAuthenticationInfo(anonymous);
         note.run(p.getId());
@@ -505,5 +506,46 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi {
         assert(formIter.next().equals("my_input"));
         assert(formIter.next().equals("my_select"));
         assert(formIter.next().equals("my_checkbox"));
+
+        // check dynamic forms values
+        String[] result = p.getResult().message().get(0).getData().split("\n");
+        assertEquals(4, result.length);
+        assertEquals("default_name", result[0]);
+        assertEquals("1", result[1]);
+        assertEquals("items: Seq[Object] = Buffer(2)", result[2]);
+        assertEquals("2", result[3]);
+    }
+
+    @Test
+    public void testPySparkZeppelinContextDynamicForms() throws IOException {
+        Note note = ZeppelinServer.notebook.createNote(anonymous);
+        Paragraph p = note.addParagraph(AuthenticationInfo.ANONYMOUS);
+        note.setName("note");
+        Map config = p.getConfig();
+        config.put("enabled", true);
+        p.setConfig(config);
+        String code = "%spark.pyspark print(z.input('my_input', 'default_name'))\n" +
+            "print(z.select('my_select', " +
+            "[('1', 'select_1'), ('2', 'select_2')], defaultValue='1'))\n" +
+            "items=z.checkbox('my_checkbox', " +
+            "[('1', 'check_1'), ('2', 'check_2')], defaultChecked=['2'])\n" +
+            "print(items[0])";
+        p.setText(code);
+        p.setAuthenticationInfo(anonymous);
+        note.run(p.getId());
+        waitForFinish(p);
+
+        assertEquals(Status.FINISHED, p.getStatus());
+        Iterator<String> formIter = p.settings.getForms().keySet().iterator();
+        assert(formIter.next().equals("my_input"));
+        assert(formIter.next().equals("my_select"));
+        assert(formIter.next().equals("my_checkbox"));
+
+        // check dynamic forms values
+        String[] result = p.getResult().message().get(0).getData().split("\n");
+        assertEquals(3, result.length);
+        assertEquals("default_name", result[0]);
+        assertEquals("1", result[1]);
+        assertEquals("2", result[2]);
     }
 }