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 <zjf...@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 <zjf...@apache.org> Authored: Tue Apr 11 18:09:35 2017 +0800 Committer: Jeff Zhang <zjf...@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]); } }