You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by ZhijieWang <gi...@git.apache.org> on 2015/06/01 20:20:42 UTC
[GitHub] incubator-zeppelin pull request: enabled download data as csv butt...
GitHub user ZhijieWang opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/89
enabled download data as csv button
I added a button to allow download the current paragraph data as csv
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ZhijieWang/incubator-zeppelin feature-download-button
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/89.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 #89
----
commit 1f7f16a08ea6d46327fc011c8e8b8bd8cbedfa8c
Author: Zhijie Wang <wa...@gmail.com>
Date: 2015-06-01T18:19:16Z
enabled download data as csv button
----
---
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] incubator-zeppelin pull request: enabled download data as csv butt...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/89#issuecomment-107772200
We are currently in the process of refactoring with the PR#56
So it should get better afterwards :)
Let me play with it a bit, we got a similar PR before, but I would rather use a non-library approach if possible.
---
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] incubator-zeppelin pull request: enabled download data as csv butt...
Posted by ZhijieWang <gi...@git.apache.org>.
Github user ZhijieWang commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/89#issuecomment-107771155
The change is to the right corner, a cloud download image. I would suggest to update documentations for how to contribute and how to build. It was quite a code diving.
And thank you for your review.
---
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] incubator-zeppelin pull request: enabled download data as csv butt...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/89#issuecomment-107770389
Hi,
Thanks for the contribution, I will review your PR.
For PR that modifies Zeppelin UI, it is better to join a screenshot so that people can see the change without building from your code.
On another note, it seems your branch is not up to date with master branch.
---
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] incubator-zeppelin pull request: enabled download data as csv butt...
Posted by ZhijieWang <gi...@git.apache.org>.
Github user ZhijieWang commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/89#issuecomment-108378381
@corneadoug Thank you for your review.
Test
>Works for table data to CSV
>Kinda works for non-csv type of data, but the file is still .csv and while my test didn't make the stack/text separated like a CSV, it could easily happen.
There is tooltip on the button
I implemented intended for csv download. I believe if the user want to download any plain text output, they can simple copy and paste.
>Cross-browser
I think we should establish a baseline -- allow user to be able to download via right click "save as" or directly launch a download prompt. With some many browsers out there, it is hard to make every feature work. Zeppelin project should have "recommended" or "supported browsers". And unfortunately I don't have IE anymore, so I can't test for that.
>Position of the button? (I actually like it there)
Feel free to move it around, i set it to show up when paragraph status == "FINISHED"
>Should the download data works only for table type of data?
I implemented intended for csv download. I believe if the user want to download any plain text output, they can simple copy and paste. Maybe when data is not in table format, the button should download the msg value and let end user to do the transformation.
>Should the CSV be quoted? Should we use a library for the transformation?
I think we should, I only had numeric value at the time of implementation. If the values are string with white space, it could cause problem.
---
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] incubator-zeppelin pull request: enabled download data as csv butt...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/89#issuecomment-108203047
Here is my test so far:
### Test
* Works for table data to CSV
* Kinda works for non-csv type of data, but the file is still .csv and while my test didn't make the stack/text separated like a CSV, it could easily happen.
* There is tooltip on the button
### Cross-browser
* Opera - Launch the download but no default name and format for the file
* Chrome - works and download as data.csv
* Firefox - Not working
* Safari - Open the file in the Browser instead of download
* IE 10 - Well somehow I can't make Zeppelin run on it anymore :p
Cross-browser for this kind of download is really a pain in the ass, that's why we tried using jquery-datatables and tabletools before.
### Discussion
* Position of the button? (I actually like it there)
* Should the download data works only for table type of data?
* Should the CSV be quoted? Should we use a library for the transformation?
---
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.
---