You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by cloverhearts <gi...@git.apache.org> on 2017/03/30 19:13:27 UTC
[GitHub] zeppelin pull request #2207: [Zeppelin-802] Support for Zeppelin Context red...
GitHub user cloverhearts opened a pull request:
https://github.com/apache/zeppelin/pull/2207
[Zeppelin-802] Support for Zeppelin Context redefinition on Python and Pyspark
### What is this PR for?
If you override the reserved word ZeppelinContext such as `z` in the python language, the whole paragraph output problem occurred.
I have taken care to avoid this issue.
`z` == `_zc` == zeppelin context
### What type of PR is it?
Improvement
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-802
### How should this be tested?
The error should not occur in the following situations:
```
%python
z = 1
print("Hello Zeppelin")
```
```
%pyspark
z = 1
print("Hello Zeppelin")
```
### Screenshots (if appropriate)
#### before
#### after
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloverhearts/zeppelin ZEPPELIN-802-pyspark-zeppelin-context
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2207.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 #2207
----
commit 7edb1ad4196f4d9e9791545ad25980727ad418db
Author: CloverHearts <cl...@gmail.com>
Date: 2017-03-30T18:26:07Z
replace name zeppelin context on pyspark
commit 2d0ea1a030109243a7d530d0e88793859b3f9a14
Author: CloverHearts <cl...@gmail.com>
Date: 2017-03-30T18:35:28Z
added test code
commit b54209da0bd60ce0ae1e46a41ed1695e5fa442b8
Author: CloverHearts <cl...@gmail.com>
Date: 2017-03-30T18:44:53Z
fix pyspark test case
commit 0eaf9a896c30037dd4e51dc18df0a2c5b9abf69c
Author: CloverHearts <cl...@gmail.com>
Date: 2017-03-30T18:46:08Z
replace name zeppelin context on python
commit f3805b3b73c533fc8dabcc39c15dfcecae0259a4
Author: CloverHearts <cl...@gmail.com>
Date: 2017-03-30T18:46:27Z
added test case on python
----
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@cloverhearts You may miss my last comment. We should separate the namespace of python process and repl, otherwise there's maintenance overhead when we update the script to add some other variables in future.
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@cloverhearts Great! Tested this patch and it works well as you described.
- Before
- After
If @astroshim feedback can also be addressed, looks GOOD to me.
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2207
Nice!!
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@cloverhearts It looks also great! \U0001f44d
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@astroshim
So what about `__zeppelin__`?
Actually, I feel similar like a python system environment word
It is less likely to be redefined by other libraries.
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
Okay, Ci pass :)
@felixcheung @zjffdu
Could you please check on this pr?
Thank you :)
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2207
LGTM
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2207
right, I think it's everything that is called after calling `exec(code)`, might have been overwritten by the user code
```
or node in to_run_exec:
mod = ast.Module([node])
code = compile(mod, '<stdin>', 'exec')
exec(code)
for node in to_run_single:
mod = ast.Interactive([node])
code = compile(mod, '<stdin>', 'single')
exec(code)
for node in to_run_hooks:
mod = ast.Module([node])
code = compile(mod, '<stdin>', 'exec')
exec(code)
```
so I think: `intp`, `output`
and perhaps `to_run_exec`, `to_run_single`, `to_run_hooks`?
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@cloverhearts I left one minor comment. Besides, we need to add document for this.
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@felixcheung Thank you for your good advice.
I will check your opinion.
and fix :)
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context red...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/2207
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
apply to namespace on python
and zeppelin context name change
before
```
z = _zc = zeppelin context
```
to
```
z = __zeppelin__ = zeppelin context
```
same
```
z.input("title", "my title")
__zeppelin__.input("title", "my title")
```
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/2207
hmm, right. Seems document is not needed \U0001f604
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
will be merging if no further comments
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
I would like to merge this.
This solves the problem of usability.
I will merge this if I no longer have any opinion.
Thank you :)
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/2207
Looks great.
one thing i worrying about is that the `_zc` can be possibly used by user easily?
how about assigning more unique key word like `__zc__` ?
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2207
we need to check if namespace work properly on py2 and py3...
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@zjffdu
Yes, I will also reflect the namespace.
Thank you for your advice :)
.
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/2207
I think a more thorough solution is to separate the namespace between python process and python repl. Only `ZeppelinContext` should be `available` in python repl namespace. It would be better to specify a space in `exec`.
e.g.
```
exec(code, global=repl_namespace)
```
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
@zjffdu
How to use is the same as the existing one.
If I add a document, should I add something like __zeppelin__ or __spark__?
What do you think? :)
---
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 #2207: [Zeppelin-802] Support for Zeppelin Context redefiniti...
Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:
https://github.com/apache/zeppelin/pull/2207
and tested environment.
| | pyspark intp | python intp |
|-|--------|------|
|python3 with matplotlib | O| O |
|python2 with matplotlib | O | O |
---
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.
---