You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by agoodm <gi...@git.apache.org> on 2016/08/18 17:54:29 UTC
[GitHub] zeppelin pull request #1343: ZEPPELIN-1328 - z.show in python interpreter do...
GitHub user agoodm opened a pull request:
https://github.com/apache/zeppelin/pull/1343
ZEPPELIN-1328 - z.show in python interpreter does not display PNG images in python 3
### What is this PR for?
Support for plotting PNG images via matplotlib inline for the python interpreter was recently added (#1329). However, these changes did not work for python3 since it handles strings differently. This PR aims to make the inline plotting compatible with both python 2 and 3.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
* [ZEPPELIN-1328](https://issues.apache.org/jira/browse/ZEPPELIN-1328)
### How should this be tested?
In a python interpreteter cell, make sure the following produce an image:
```python
%python
import matplotlib.pyplot as plt
import numpy as np
x = np.arange(5)
plt.plot(x)
z.show(plt, fmt='png') # Repeat for fmt='svg'
```
This should be tested for both python2 and 3 interpreters (via the interpreter settings page).
### 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/agoodm/zeppelin ZEPPELIN-1328
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1343.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 #1343
----
----
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/1343
LGTM. is it easy to add automated unit test 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 pull request #1343: ZEPPELIN-1328 - z.show in python interpreter do...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1343
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1343
Thank you @agoodm for taking care, looks great to me.
I think it sets an excellent example of Python version compatibility.
Will merge to master if there is no further discussion
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by bustios <gi...@git.apache.org>.
Github user bustios commented on the issue:
https://github.com/apache/zeppelin/pull/1343
@agoodm your solution looks good, but the `import io` is going to generate an error in the `show_dataframe` method for Python 2. Running the tests can help you to catch this error. It would be better to use:
```python
from io import BytesIO
```
before (and not to delete):
```python
try:
import StringIO as io
except ImportError:
import io as io
```
another option could be:
```python
from io import BytesIO
try:
from StringIO import StringIO
except ImportError:
from io import StringIO
```
and replage `io.StringIO` by `StringIO`.
Personally, I prefer the second option because it has the advantage of avoiding the `io` package to be hidden by `import StringIO as io` for future use.
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by agoodm <gi...@git.apache.org>.
Github user agoodm commented on the issue:
https://github.com/apache/zeppelin/pull/1343
@bzz @bustios I tested the changes in a notebook for both python2/3 as well as ran the unit tests. Hopefully we are good to go this time.
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1343
There actually are tests, but we do not have CI profiles with both Python2 and Python3 yet. I think, I will take a look into creating one soon.
Though @felixcheung is right, there are no Integration Tests for Python interpreter yet, the one that might verify that image appears in the paragraph. It would be nice to have one, but I guess it's a bit out of scope for this PR.
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by agoodm <gi...@git.apache.org>.
Github user agoodm commented on the issue:
https://github.com/apache/zeppelin/pull/1343
@bustios Thanks for the catch. I agree that the second option you posted would be a lot cleaner, so I'll go ahead and test it.
---
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 #1343: ZEPPELIN-1328 - z.show in python interpreter does not ...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1343
Looks great to me, thank you for taking care @agoodm !
Merging to master if there is no further discussion.
[CI failure](https://s3.amazonaws.com/archive.travis-ci.org/jobs/153350047/log.txt) is not relevant and is due to flaky `RemoteInterpreterTest` test
```
Running org.apache.zeppelin.interpreter.remote.RemoteInterpreterTest
....
18:31:23,153 INFO org.apache.zeppelin.interpreter.remote.RemoteInterpreterManagedProcess:116 - Run interpreter process [/home/travis/build/apache/zeppelin/zeppelin-interpreter/../bin/interpreter.sh, -d, fake, -p, 50714, -l, fakeRepo]
18:31:23,660 INFO org.apache.zeppelin.interpreter.remote.RemoteInterpreter:223 - Create remote interpreter org.apache.zeppelin.interpreter.remote.mock.MockInterpreterA
18:31:23,906 INFO org.apache.zeppelin.interpreter.remote.RemoteInterpreter:223 - Create remote interpreter org.apache.zeppelin.interpreter.remote.mock.MockInterpreterB
18:31:23,945 ERROR org.apache.zeppelin.interpreter.remote.RemoteInterpreterEventPoller:192 - Can't handle event RemoteInterpreterEvent(type:OUTPUT_UPDATE, data:{"data":"","noteId":"note","paragraphId":"id"})
java.lang.NullPointerException
at org.apache.zeppelin.interpreter.remote.RemoteInterpreterEventPoller.run(RemoteInterpreterEventPoller.java:174)
18:31:23,986 ERROR org.apache.zeppelin.interpreter.remote.RemoteInterpreterEventPoller:192 - Can't handle event RemoteInterpreterEvent(type:OUTPUT_APPEND, data:{"data":"","noteId":"note","paragraphId":"id"})
java.lang.NullPointerException
at org.apache.zeppelin.interpreter.remote.RemoteInterpreterEventPoller.run(RemoteInterpreterEventPoller.java:160)
18:31:23,999 INFO org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcess:127 - shutdown interpreter process
18:31:26,114 INFO org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcess:137 - Exception in RemoteInterpreterProcess while synchronized dereference, can safely ignore exception while client.shutdown() may terminates remote process
18:31:26,116 INFO org.apache.zeppelin.interpreter.remote.RemoteInterpreterManagedProcess:154 - Interpreter process exited 0
18:31:26,616 INFO org.apache.zeppelin.interpreter.InterpreterGroup:140 - Close interpreter group InterpreterGroup_1471545083152_-1987997814
18:31:26,634 INFO org.apache.zeppelin.interpreter.InterpreterGroup:204 - Destroy interpreter group InterpreterGroup_1471545083152_-1987997814
```
---
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.
---