You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zhongneu <gi...@git.apache.org> on 2016/02/14 06:58:27 UTC
[GitHub] incubator-zeppelin pull request: Exporting to TSV
GitHub user zhongneu opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/714
Exporting to TSV
### What is this PR for?
Allow users to export data in a paragraph to a TSV file.
There is already a [PR](https://github.com/apache/incubator-zeppelin/pull/6) for this, but it uses DataTables, which requires Flash.
### What type of PR is it?
Feature
### Todos
* [ ] - Hide/disable the button if it is not in table/chart view
* [ ] - Find an icon for the new button
### Is there a relevant Jira issue?
[ZEPPELIN-672](https://issues.apache.org/jira/browse/ZEPPELIN-672)
### How should this be tested?
1. create a paragraph with data in %table view
2. select the "Export to TSV" item in the paragraph settings list
### Screenshots (if appropriate)
### 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/zhongneu/incubator-zeppelin export-to-tsv
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/714.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 #714
----
commit 2281c40d70e1c7907e8aa18ae8b54646382979a8
Author: Zhong Wang <wa...@gmail.com>
Date: 2016-02-14T05:41:51Z
first attempt to implement exporting to TSV
----
---
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: Exporting to TSV
Posted by ankurmitujjain <gi...@git.apache.org>.
Github user ankurmitujjain commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/714#issuecomment-211921573
@corneadoug I think its not right/user friendly way to have download data functionality.
You liked approach of including download buttons in "settings".
But I believe **"Settings" is something to provide configuration/setting options and not to provide any functionality**.
Also @adejanovski [#6](https://github.com/apache/incubator-zeppelin/pull/6#issuecomment-185086161) commented rightly that
> viewers have to find out that they need to click on the gear, which is hidden in report mode and simply absent when you link directly to the paragraph as an iframe
So I believe it's not a good idea to include buttons in settings as many community member don't liked it.
Secondly if DataTables provides export functionality which works and is extensible, than we should use it instead of writing our own way to do data export, this I mentioned earlier as well... [#6](https://github.com/apache/incubator-zeppelin/pull/6#issuecomment-185081090)
Also about extra UI space, its not an issue for using datatable or any of its functionality, but its general issue and to fix it we need to have a different PR...
If I look different notebook implementation, than the way they handle this issue is like below...
They are providing all buttons at bottom of table, and zeppelin provides same on top of table.
---
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: Exporting to TSV
Posted by johnnyws <gi...@git.apache.org>.
Github user johnnyws commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/714#issuecomment-211792955
@corneadoug are you sure you want to merge this? I thought we were working on #761?
There are still pending TODOs in this PR. I don't think we should merge it before finish them.
---
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: Exporting to TSV
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/714#issuecomment-211799196
It was already discussed there.
Regarding the TODO, there is only one that is useful: `Hide/disable the button if it is not in table/chart view`, but it can always be done later. (not critical for now)
The icon is fine, the Safari problem is not related to this feature (it affect also the existing export notebook), the test with large dataset doesn't matter since if it was too big, the browser will crash.
If we get this done with, then we can work on improving it, and build around it.
Otherwise it just become a blocker
---
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 #714: Exporting to TSV
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/714
---
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 issue #714: Exporting to TSV
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/incubator-zeppelin/pull/714
Merging this if no more discussions.
I will take care of moving the button as suggested next to the graph buttons with a download icon, right after.
---
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: Exporting to TSV
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/714#issuecomment-211207176
Tested after rebasing with master, no conflicts.
Let's rebase it and Merge 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] incubator-zeppelin pull request: Exporting to TSV
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/714#issuecomment-211785324
@zhongneu Could you rebase?
---
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.
---