[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16050963#comment-16050963 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue closed the pull request at: https://github.com/apache/incubator-toree/pull/104 > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16049594#comment-16049594 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r122049346 --- Diff: kernel-api/src/main/scala/org/apache/toree/interpreter/broker/BrokerTransformer.scala --- @@ -43,7 +43,7 @@ class BrokerTransformer { import scala.concurrent.ExecutionContext.Implicits.global futureResult - .map(results => (Results.Success, Left(results))) + .map(results => (Results.Success, Left(Map("text/plain" -> results --- End diff -- Opened https://issues.apache.org/jira/browse/TOREE-418 > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16049591#comment-16049591 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r122048356 --- Diff: kernel/src/test/scala/org/apache/toree/kernel/protocol/v5/relay/ExecuteRequestRelaySpec.scala --- @@ -88,7 +89,7 @@ class ExecuteRequestRelaySpec extends TestKit( // Expected does not actually match real return of magic, which // is a tuple of ExecuteReply and ExecuteResult val expected = new ExecuteAborted() -interpreterActorProbe.expectMsgClass( +interpreterActorProbe.expectMsgClass(max = Duration(5, TimeUnit.SECONDS), --- End diff -- Opened https://issues.apache.org/jira/browse/TOREE-417 > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16049589#comment-16049589 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r122047867 --- Diff: kernel/src/test/scala/org/apache/toree/kernel/protocol/v5/stream/KernelInputStreamSpec.scala --- @@ -65,6 +65,9 @@ class KernelInputStreamSpec // set of data doReturn(system.actorSelection(fakeInputOutputHandlerActor.path.toString)) .when(mockActorLoader).load(MessageType.Incoming.InputReply) +// Allow time for the actors to start. This avoids read() hanging forever +// when running tests in gradle. +Thread.sleep(100) --- End diff -- Opened https://issues.apache.org/jira/browse/TOREE-416 > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16049579#comment-16049579 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r122046211 --- Diff: kernel-api/src/main/scala/org/apache/toree/interpreter/broker/BrokerTransformer.scala --- @@ -43,7 +43,7 @@ class BrokerTransformer { import scala.concurrent.ExecutionContext.Implicits.global futureResult - .map(results => (Results.Success, Left(results))) + .map(results => (Results.Success, Left(Map("text/plain" -> results --- End diff -- No, kernel-api doesn't depend on protocol so it isn't available. But I think that this code shouldn't be sending mime types in the first place. Sending text/plain here happens because we haven't updated the Python kernel to correctly inspect its objects. Eventually, the python interpreter should send back a result that is a py4j reference to the final value and the python representation (from _repr_ and _repr_html_) or a JVM object and None to indicate that the Jupyter JVM displayer should be called. So for now, I'd rather not change module dependencies. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048247#comment-16048247 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121765250 --- Diff: scala-interpreter/build.sbt --- @@ -18,3 +18,4 @@ import sbt.Tests.{Group, SubProcess} */ libraryDependencies ++= Dependencies.sparkAll.value +libraryDependencies += "com.github.jupyter" % "jvm-repr" % "0.1.0" --- End diff -- Yes, we probably should. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048245#comment-16048245 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121765086 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaDisplayers.scala --- @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package org.apache.toree.kernel.interpreter.scala + +import java.util + +import scala.collection.JavaConverters._ +import scala.collection.mutable +import scala.util.Try +import org.apache.spark.SparkContext +import org.apache.spark.sql.Row +import org.apache.spark.sql.SparkSession +import jupyter.Displayer +import jupyter.Displayers +import jupyter.MIMETypes +import org.apache.toree.kernel.protocol.v5.MIMEType +import org.apache.toree.magic.MagicOutput + +object ScalaDisplayers { + + def ensureLoaded(): Unit = () --- End diff -- Will do. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048208#comment-16048208 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121758601 --- Diff: scala-interpreter/build.sbt --- @@ -18,3 +18,4 @@ import sbt.Tests.{Group, SubProcess} */ libraryDependencies ++= Dependencies.sparkAll.value +libraryDependencies += "com.github.jupyter" % "jvm-repr" % "0.1.0" --- End diff -- Given that all of the interpreter output gets funneled through the JVM before being sent back, would it make more sense for us to use this library at the JVM level for all of them? Either way, seems like another issue to open for discussion. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048207#comment-16048207 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121758304 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaDisplayers.scala --- @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package org.apache.toree.kernel.interpreter.scala + +import java.util + +import scala.collection.JavaConverters._ +import scala.collection.mutable +import scala.util.Try +import org.apache.spark.SparkContext +import org.apache.spark.sql.Row +import org.apache.spark.sql.SparkSession +import jupyter.Displayer +import jupyter.Displayers +import jupyter.MIMETypes +import org.apache.toree.kernel.protocol.v5.MIMEType +import org.apache.toree.magic.MagicOutput + +object ScalaDisplayers { + + def ensureLoaded(): Unit = () --- End diff -- Can you add a doc to explain that reason? Otherwise, I might forget later and try to remove it. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048204#comment-16048204 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121757522 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaDisplayers.scala --- @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package org.apache.toree.kernel.interpreter.scala + +import java.util + +import scala.collection.JavaConverters._ +import scala.collection.mutable +import scala.util.Try +import org.apache.spark.SparkContext +import org.apache.spark.sql.Row +import org.apache.spark.sql.SparkSession +import jupyter.Displayer +import jupyter.Displayers +import jupyter.MIMETypes +import org.apache.toree.kernel.protocol.v5.MIMEType +import org.apache.toree.magic.MagicOutput + +object ScalaDisplayers { + + def ensureLoaded(): Unit = () --- End diff -- Calling this ensures the `ScalaDisplayers` object has been loaded, which performs the registration. Without calling this to make sure it loads, the registration further down never happens and we don't get representations. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048205#comment-16048205 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121757635 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaInterpreter.scala --- @@ -18,30 +18,34 @@ package org.apache.toree.kernel.interpreter.scala import java.io.ByteArrayOutputStream -import java.net.{URL, URLClassLoader} -import java.nio.charset.Charset import java.util.concurrent.ExecutionException - import com.typesafe.config.{Config, ConfigFactory} +import jupyter.Displayers import org.apache.spark.SparkContext import org.apache.spark.sql.SparkSession import org.apache.spark.repl.Main - import org.apache.toree.interpreter._ -import org.apache.toree.kernel.api.{KernelLike, KernelOptions} +import org.apache.toree.kernel.api.KernelLike import org.apache.toree.utils.TaskManager import org.slf4j.LoggerFactory import org.apache.toree.kernel.BuildInfo - +import org.apache.toree.kernel.protocol.v5.MIMEType import scala.annotation.tailrec +import scala.collection.JavaConverters._ import scala.concurrent.{Await, Future} import scala.language.reflectiveCalls import scala.tools.nsc.Settings import scala.tools.nsc.interpreter.{IR, OutputStream} import scala.tools.nsc.util.ClassPath -import scala.util.{Try => UtilTry} +import scala.util.matching.Regex class ScalaInterpreter(private val config:Config = ConfigFactory.load) extends Interpreter with ScalaInterpreterSpecific { + import ScalaInterpreter._ + + ScalaDisplayers.ensureLoaded() --- End diff -- This makes sure that the registration in `ScalaDisplayers` happens when that class loads. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048203#comment-16048203 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121757204 --- Diff: scala-interpreter/build.sbt --- @@ -18,3 +18,4 @@ import sbt.Tests.{Group, SubProcess} */ libraryDependencies ++= Dependencies.sparkAll.value +libraryDependencies += "com.github.jupyter" % "jvm-repr" % "0.1.0" --- End diff -- I think we'll need to take a look after this PR at the other interpreters. PySpark, for example, should adhere to the common python convention to use _repr_html_ for objects that exist in the python process, and should use the Jupyter library for objects that are resident in the JVM (like DataFrames). The background on Jupyter's jvm-repr library is that I'm trying to work with the Jupyter community to standardize a system for libraries and kernels to collaborate on rich representations for the JVM. Vegas, for example, should register a function to display its objects so it doesn't have to be built into the kernel or bolted on by users. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048202#comment-16048202 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121756270 --- Diff: kernel/src/test/scala/org/apache/toree/kernel/protocol/v5/relay/ExecuteRequestRelaySpec.scala --- @@ -88,7 +89,7 @@ class ExecuteRequestRelaySpec extends TestKit( // Expected does not actually match real return of magic, which // is a tuple of ExecuteReply and ExecuteResult val expected = new ExecuteAborted() -interpreterActorProbe.expectMsgClass( +interpreterActorProbe.expectMsgClass(max = Duration(5, TimeUnit.SECONDS), --- End diff -- The default is 3 seconds, but it was flaky for me. This was another work-around that probably shouldn't be here. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048201#comment-16048201 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121756111 --- Diff: kernel/src/test/scala/org/apache/toree/kernel/protocol/v5/stream/KernelInputStreamSpec.scala --- @@ -65,6 +65,9 @@ class KernelInputStreamSpec // set of data doReturn(system.actorSelection(fakeInputOutputHandlerActor.path.toString)) .when(mockActorLoader).load(MessageType.Incoming.InputReply) +// Allow time for the actors to start. This avoids read() hanging forever +// when running tests in gradle. +Thread.sleep(100) --- End diff -- Sounds good. Go with that. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048193#comment-16048193 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121753408 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaDisplayers.scala --- @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package org.apache.toree.kernel.interpreter.scala + +import java.util + +import scala.collection.JavaConverters._ +import scala.collection.mutable +import scala.util.Try +import org.apache.spark.SparkContext +import org.apache.spark.sql.Row +import org.apache.spark.sql.SparkSession +import jupyter.Displayer +import jupyter.Displayers +import jupyter.MIMETypes +import org.apache.toree.kernel.protocol.v5.MIMEType +import org.apache.toree.magic.MagicOutput + +object ScalaDisplayers { + + def ensureLoaded(): Unit = () --- End diff -- This does nothing? Why is this method here? Should we have a common interface that the `ScalaDisplayers` object inherits? > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048195#comment-16048195 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121751667 --- Diff: kernel/src/test/scala/org/apache/toree/kernel/protocol/v5/relay/ExecuteRequestRelaySpec.scala --- @@ -88,7 +89,7 @@ class ExecuteRequestRelaySpec extends TestKit( // Expected does not actually match real return of magic, which // is a tuple of ExecuteReply and ExecuteResult val expected = new ExecuteAborted() -interpreterActorProbe.expectMsgClass( +interpreterActorProbe.expectMsgClass(max = Duration(5, TimeUnit.SECONDS), --- End diff -- Rather than hard coding the duration to 5 seconds, we should have a constant somewhere defined to that duration. Furthermore, I'm trying to remember how akka's testkit time scaling works. I honestly can't remember, but we want to make sure that it does indeed scale according to our config. I think testkit does that for us, so this should be fine, but we probably need to double-check. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048194#comment-16048194 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121753548 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaInterpreter.scala --- @@ -18,30 +18,34 @@ package org.apache.toree.kernel.interpreter.scala import java.io.ByteArrayOutputStream -import java.net.{URL, URLClassLoader} -import java.nio.charset.Charset import java.util.concurrent.ExecutionException - import com.typesafe.config.{Config, ConfigFactory} +import jupyter.Displayers import org.apache.spark.SparkContext import org.apache.spark.sql.SparkSession import org.apache.spark.repl.Main - import org.apache.toree.interpreter._ -import org.apache.toree.kernel.api.{KernelLike, KernelOptions} +import org.apache.toree.kernel.api.KernelLike import org.apache.toree.utils.TaskManager import org.slf4j.LoggerFactory import org.apache.toree.kernel.BuildInfo - +import org.apache.toree.kernel.protocol.v5.MIMEType import scala.annotation.tailrec +import scala.collection.JavaConverters._ import scala.concurrent.{Await, Future} import scala.language.reflectiveCalls import scala.tools.nsc.Settings import scala.tools.nsc.interpreter.{IR, OutputStream} import scala.tools.nsc.util.ClassPath -import scala.util.{Try => UtilTry} +import scala.util.matching.Regex class ScalaInterpreter(private val config:Config = ConfigFactory.load) extends Interpreter with ScalaInterpreterSpecific { + import ScalaInterpreter._ + + ScalaDisplayers.ensureLoaded() --- End diff -- Why are we calling this if it does nothing? > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048197#comment-16048197 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121752852 --- Diff: scala-interpreter/build.sbt --- @@ -18,3 +18,4 @@ import sbt.Tests.{Group, SubProcess} */ libraryDependencies ++= Dependencies.sparkAll.value +libraryDependencies += "com.github.jupyter" % "jvm-repr" % "0.1.0" --- End diff -- What does this do? Looking it up, I see that @rdblue was the author, but now it's under the jupyter umbrella. BSD-3 clause should be fine. Is there a reason it's just used here and not all interpreters? > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16048198#comment-16048198 ] ASF GitHub Bot commented on TOREE-380: -- Github user chipsenkbeil commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r121751964 --- Diff: kernel/src/test/scala/org/apache/toree/kernel/protocol/v5/stream/KernelInputStreamSpec.scala --- @@ -65,6 +65,9 @@ class KernelInputStreamSpec // set of data doReturn(system.actorSelection(fakeInputOutputHandlerActor.path.toString)) .when(mockActorLoader).load(MessageType.Incoming.InputReply) +// Allow time for the actors to start. This avoids read() hanging forever +// when running tests in gradle. +Thread.sleep(100) --- End diff -- I _really_ dislike `sleep` calls. Is there any better way to do this? Is there any flag or way to know when the actors have started and use a future, callback, or something here rather than a thread sleep? > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16045690#comment-16045690 ] ASF GitHub Bot commented on TOREE-380: -- Github user rdblue closed the pull request at: https://github.com/apache/incubator-toree/pull/104 > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue >Assignee: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TOREE-380) Interpreters should be allowed to send results other than text/plain.
[ https://issues.apache.org/jira/browse/TOREE-380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15936608#comment-15936608 ] ASF GitHub Bot commented on TOREE-380: -- Github user kevin-bates commented on a diff in the pull request: https://github.com/apache/incubator-toree/pull/104#discussion_r107460553 --- Diff: scala-interpreter/src/main/scala/org/apache/toree/kernel/interpreter/scala/ScalaInterpreter.scala --- @@ -18,37 +18,45 @@ package org.apache.toree.kernel.interpreter.scala import java.io.ByteArrayOutputStream -import java.net.{URL, URLClassLoader} -import java.nio.charset.Charset import java.util.concurrent.ExecutionException import com.typesafe.config.{Config, ConfigFactory} import org.apache.spark.SparkContext -import org.apache.spark.sql.SparkSession import org.apache.spark.repl.Main +import org.apache.spark.sql.SparkSession import org.apache.toree.interpreter._ -import org.apache.toree.kernel.api.{KernelLike, KernelOptions} +import org.apache.toree.kernel.api.KernelLike import org.apache.toree.utils.{MultiOutputStream, TaskManager} import org.slf4j.LoggerFactory import org.apache.toree.kernel.BuildInfo import scala.annotation.tailrec +import scala.collection.mutable import scala.concurrent.{Await, Future} import scala.language.reflectiveCalls import scala.tools.nsc.Settings import scala.tools.nsc.interpreter.{IR, OutputStream} import scala.tools.nsc.util.ClassPath -import scala.util.{Try => UtilTry} + +import org.apache.toree.kernel.protocol.v5.MIMEType +import org.apache.toree.magic.MagicOutput +import vegas.DSL.ExtendedUnitSpecBuilder +import vegas.render.StaticHTMLRenderer + class ScalaInterpreter(private val config:Config = ConfigFactory.load) extends Interpreter with ScalaInterpreterSpecific { + import ScalaInterpreter._ + + private var kernel: KernelLike = _ + protected val logger = LoggerFactory.getLogger(this.getClass.getName) protected val _thisClassloader = this.getClass.getClassLoader protected val lastResultOut = new ByteArrayOutputStream() - protected val multiOutputStream = MultiOutputStream(List(Console.out, lastResultOut)) + protected val multiOutputStream = lastResultOut --- End diff -- At a minimum, this needs to remain a MultiOutputStream (of one item) else it will produce a compilation error in [here](https://github.com/apache/incubator-toree/blob/master/scala-interpreter/src/test/scala/integration/interpreter/scala/AddExternalJarMagicSpecForIntegration.scala#L41). Removal of Console.out is further discussed in #116. > Interpreters should be allowed to send results other than text/plain. > - > > Key: TOREE-380 > URL: https://issues.apache.org/jira/browse/TOREE-380 > Project: TOREE > Issue Type: Improvement >Reporter: Ryan Blue > > Jupyter allows kernels to send results using different content types, but > this isn't allowed by Toree for its interpreters. This prevents custom > display logic. The basic problem is that {{ExecuteOutput}} is a {{String}} > and not a {{Map[String, String]}} like {{CellMagicOutput}}. -- This message was sent by Atlassian JIRA (v6.3.15#6346)