[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-24 Thread felixcheung
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1352 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

[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-23 Thread bzz
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1352 Looks great to me! Merging 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

[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-23 Thread agoodm
Github user agoodm commented on the issue: https://github.com/apache/zeppelin/pull/1352 @bustios Nice job, this works great now. 👍 --- 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

[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-23 Thread agoodm
Github user agoodm commented on the issue: https://github.com/apache/zeppelin/pull/1352 @bustios Nice job on removing the need to perform the python version check completely. This certainly helps make the code look cleaner. However I cannot get your changes to work

[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-23 Thread bustios
Github user bustios commented on the issue: https://github.com/apache/zeppelin/pull/1352 Done. I added a comment and put the decoding statement in a separate line for a better understanding. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-22 Thread bzz
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1352 Got it, makes perfect sense! Do you think we should add a comment in there, explaining that compatibility is the reason for such decoding? I feel like this might help future contributors to

[GitHub] zeppelin issue #1352: [ZEPPELIN-1327] Fix bug in z.show for Python interpret...

2016-08-22 Thread bustios
Github user bustios commented on the issue: https://github.com/apache/zeppelin/pull/1352 Yes @bzz, I just deleted `if self.py3:` but not `img_str = img_str.decode('ascii')` (it is in line 173), which means it will be executed for both Python 2 and 3. Decoding bytes to string it