You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by echarles <gi...@git.apache.org> on 2017/04/05 17:47:24 UTC
[GitHub] zeppelin pull request #2227: [ZEPPELIN-2359] Support Spell as Display
GitHub user echarles opened a pull request:
https://github.com/apache/zeppelin/pull/2227
[ZEPPELIN-2359] Support Spell as Display
### What is this PR for?
SPELL should be considered as another display type, just like TEXT, HTML, TABLE...
### What type of PR is it?
[Improvement]
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2359
### How should this be tested?
Run in a paragraph
```
val json = """{"array":[1,2,3],"boolean":true,"null":null,"number":123,"object":{"a":"b","c":"d","e":"f"},"string":"Hello World"}"""
println(s"""%spell %json
$json""")
```
or
```
val hello = "hello all"
println(s"""%spell %echo
$hello""")
```
### Screenshots
NA
### Questions:
* Does the licenses files need update? N
* Is there breaking changes for older versions? N
* Does this needs documentation? Y
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/datalayer/zeppelin ZEPPELIN-2359
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2227.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2227
----
commit ebe7ebb336f182581df5e2c5d7df01308f88b367
Author: Eric Charles <er...@datalayer.io>
Date: 2017-03-18T11:23:13Z
Automatic message for commit of samedi 18 mars 2017, 11:23:13 (UTC+0000)
commit 39543dec69555ec9968175ad6f8a36dcb7a28ae0
Author: Eric Charles <er...@datalayer.io>
Date: 2017-03-25T13:03:02Z
Merge branch 'master' of https://github.com/apache/zeppelin
commit e89fd7098c11a821bbb78498f627eada0700992d
Author: Eric Charles <er...@datalayer.io>
Date: 2017-03-29T04:00:14Z
Merge branch 'master' of https://github.com/apache/zeppelin
commit 2fb48c0a8db6e5bd154861133724e0d0947bf792
Author: Eric Charles <er...@datalayer.io>
Date: 2017-04-05T13:16:40Z
Merge branch 'master' of https://github.com/apache/zeppelin
commit 78d0ff2d92de4cb0027c7ea78be8db1d6cc4cf8e
Author: Eric Charles <er...@datalayer.io>
Date: 2017-04-05T16:15:55Z
revert to scala 2.10
commit 4c1d5b0f8b7d8ce61eacf56aedefcc877539f5e3
Author: Eric Charles <er...@datalayer.io>
Date: 2017-04-05T17:40:43Z
Support Spell as Display
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/2227
@1ambda I have added the log. Regarding you second comment, I am not sure what you mean. Is there something I should change? Is it not working as expected?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/2227
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/2227
@1ambda thx a lot for the follow-up and your great work.
This branch now supports what we want:
```
println("""%json {"array":[1,2,3],"boolean":true,"null":null,"number":123,"object":{"a":"b","c":"d","e":"f"},"string":"Hello World"}""")
```
I would be great if you could test it and give me any feedback.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2227
Usage of existing display systems is `%DISPLAY_SYSTEM_NAME` (e.g. `%table`, `%html`). Will where be a way to keep usage the same whether display system is installed by Spell or not?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/2227
@Leemoonsoo @1ambda I see your point and agree with it.
However, we need to decide which InterpreterResult to return. For now it can be { TABLE, IMG, SVG, NULL }. The Java code needs to decide which value to return to the Javascript, and the Javascript based on that value needs to take decisions (display as a known display-type, or render code via Spell).
For now, I don't see how to implement / recognize on Java side the InterpreterResult to return.
Can you describe any option you have in mind?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/2227
Thanks for contribution. @echarles
It would be nice to keep same usage for spell even with backend display system. That's the original intention (https://github.com/apache/zeppelin/pull/1940). Here are few reasons.
- Users might not want to learn new usage for just using display system.
- Even some users don't care it's spell or not.
- Sometimes, we need complex implementation to provide simple usage for users. I believe this is the case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/2227
Hi @echarles. Sorry for late comment.
I agree it's quite hard to implement. Since `InterpreterOutput` class is widely used.
1. We need to add a variable to `RemoteInterpreterContext` which is a thrift generated class to pass currently available spell magics so that interpreter can use it for their `InterpreterOutput`
2. Since InterpreterOutput usage depends on interpreter type, we might use static, synchronized variable in `InterpreterOutput` both sides in `ZeppelinServer` and `Interpreter`.
This is what i am thinking about.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/2227
@Leemoonsoo I wanted to introduce minimum changes in javascript and stay aligned with the idea of interpreters returning different display types. Introducing a new `spell` display has sounded logic and easy to me. But I agree that in this case, user has still to add a second, potentially confusing, magic keyword to define the targeted spell.
The other way would be to consider that if interpreter returns a value that begins with `%`, we consider it as a spell... @1ambda WDYT?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2227: [ZEPPELIN-2359] Support Spell as Display
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2227
\cc @1ambda
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---