ZEPPELIN-830 Improve table display to handle large data #858

r-kamath · 2016-04-26T12:10:10Z

What is this PR for?

This is an improvement for table display. By using Handsontable we can load and display more data in paragraphs. Tested using sample tabular data consisting 10000x80 cells

What type of PR is it?

Improvement

Todos

- Decimal formatting
- Rendering without angular/html directives
- UI fixes
- License doc update

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-830

How should this be tested?

TODO : test cases to follow

Screenshots (if appropriate)

Questions:

Does the licenses files need update? Yes
Is there breaking changes for older versions? No
Does this needs documentation? Maybe

prabhjyotsingh · 2016-04-26T13:52:00Z

@r-kamath build fails with

Failed tests: 
  SparkParagraphIT.testSqlSpark:169 Paragraph from SparkParagraphIT of testSqlSpark result: 
Expected: "age job marital education balance\n30 unemployed married primary 1,787"
     but: was "age\njob\nmarital\neducation\nbalance\n30 unemployed married primary 1787\nage\njob\nmarital\neducation\nbalance"

Looks like this require selenium test update.

astroshim · 2016-04-26T14:35:53Z

I passed the build with no problems and functionality is working very well.
but @r-kamath you should make the green CI.
Thanks.

felixcheung · 2016-04-26T18:49:07Z

How does this work with some of the pending data table changes?
like #6, #714, #725, #761

r-kamath · 2016-04-27T10:16:21Z

Won't affect #714 and #725

#6 and #761 are using jQuery Datatables
This PR is a proposal to use Handsontable instead of datatables. Handsontable is more like a spreadsheet and has good performance with many builtin features and plugin support.

I've done a basic performance test with 20000x80 rows and columns data and few paragraphs with a subset.
PR #761 : https://drive.google.com/open?id=0B8L8gc9P-C-6eEhHeFVRd3cxNEE
PR #858 : https://drive.google.com/open?id=0B8L8gc9P-C-6dEo3OWVWa1J3QTQ

felixcheung · 2016-04-27T20:52:29Z

We would need export to file feature which seems like is not supported in the free Handsontable version?

r-kamath · 2016-04-28T04:45:14Z

Right, it is supported only in pro ver. Handsontable's Export feature is in front-end. IMHO export should be a backend feature and will be a good addition to our Notebook REST api.

Leemoonsoo · 2016-04-28T18:07:50Z

zeppelin-web/src/index.html

+    <script src="bower_components/zeroclipboard/dist/ZeroClipboard.js"></script>
+    <script src="bower_components/moment/moment.js"></script>
+    <script src="bower_components/pikaday/pikaday.js"></script>
+    <script src="bower_components/handsontable/dist/handsontable.js"></script>


pickaday.js, moment.js, ZeroClipboard.js, pikaday.css are they required to use handsontable?

ZeroClipboard and pikaday.js are not required, need to see how to remove them from the build.
As of now moment.js is used only for number formatting (7bb508e)

I was wrong. All three deps are required. Use without pikaday #2841

Leemoonsoo · 2016-04-28T18:15:57Z

Is it possible to handle '%html' inside of table cell?

if i run

println(s"""%table key\tvalue
html\t%html <dev style='color:blue'>blue</div>
""")

Master branch

This PR

r-kamath · 2016-04-28T18:22:04Z

@Leemoonsoo that's in the todo list. Can be done either in FE or in server. Thoughts?

r-kamath · 2016-04-29T09:47:04Z

@Leemoonsoo %html rendering is fixed in a96c94a

Leemoonsoo · 2016-04-30T03:31:57Z

%html rendering is working nicely! If pickaday.js, moment.js, ZeroClipboard.js are required to use handsontable, they also need to be in 'zeppelin-distribution/src/bin_license/LICENSE'

…into ZEPPELIN-830

r-kamath · 2016-05-02T07:19:07Z

@Leemoonsoo license doc updated and rebased 2e422ea 4fbb22b

prabhjyotsingh · 2016-05-04T16:13:22Z

LGTM! nice improvement 👍

Leemoonsoo · 2016-05-04T18:07:38Z

@r-kamath Are there any more works left for unchecked todo item 'UI fixes', in the description of this PR?

r-kamath · 2016-05-04T18:14:38Z

@Leemoonsoo the only pending item is the sort header arrow style. Should we try something more stylish ?

Leemoonsoo · 2016-05-04T18:23:06Z

Arrow style looks fine. I think it's good to be merged!

r-kamath · 2016-05-04T18:26:08Z

@Leemoonsoo cool. I have checked the 'UI fixes'. thanks

Leemoonsoo · 2016-05-04T18:29:23Z

Last CI test failure is unrelated to the change. LGTM.

felixcheung · 2016-05-04T21:15:58Z

does this PR change the export to CSV capability?

corneadoug · 2016-05-05T00:35:57Z

@felixcheung No, There is still no export to CSV feature

r-kamath · 2016-05-05T07:12:07Z

@felixcheung this PR is only for improving table performance. CSV/TSV export is in #725 and #714

prabhjyotsingh · 2016-05-09T13:32:05Z

@r-kamath can you resolve branch conflict.

…into ZEPPELIN-830

r-kamath · 2016-05-10T04:33:32Z

@prabhjyotsingh conflict resolved f13ba91

prabhjyotsingh · 2016-05-10T06:48:53Z

Thanks @r-kamath for contributing towards front-end performance.
Merging if no more discussion.

corneadoug · 2016-05-10T11:31:10Z

Anyway to not have:

zeroclipboard
pikaday
moment

included when installing Handsontable?

r-kamath · 2016-05-10T12:08:25Z

@corneadoug Use without pikaday #2841

corneadoug · 2016-05-10T12:15:09Z

@r-kamath https://github.com/handsontable/hot-builder
I understand that by default it seems to be needed, however I'm reticent to include 3 different dependencies that we are not using.

r-kamath · 2016-05-10T12:28:05Z

Not sure if we can avoid those dependencies by choosing hot-builder!
Though we are not using the copy/ paste, date picker and date formatting, it is still a feature in Handsontable.
IMO we shouldn't be worried about those three dependencies because it is not a part of our web bower.json

prabhjyotsingh · 2016-05-10T13:56:42Z

I would agree with @r-kamath, as long as those dependencies are not been written in bower.json we should be ok.
https://github.com/apache/incubator-zeppelin/pull/858/files#diff-6bcc7568e5b6af0b1499ab172fc66af4L33

corneadoug · 2016-05-10T14:56:03Z

I don't get why its not important to have unused additional dependencies in our build, as long as its not in bower.json, maybe somebody can explain more.

r-kamath · 2016-05-10T15:16:18Z

@corneadoug Again, this is an issue on handsontable please follow these links. Those entries in index.html should disappear when handsontable team fix the issue.
handsontable/handsontable#2841
handsontable/handsontable#2974

Also it is included in the Improvement-suggestions section.

More details here : https://github.com/handsontable/handsontable

@Leemoonsoo @prabhjyotsingh what say?

corneadoug · 2016-05-12T07:33:08Z

@r-kamath I don't want to make this PR wait forever.
If it was easy to do, I would prefer removing those dependencies. Mainly because everytime we want to add a dependency, we should ask ourselves if its really necessary, so adding 3 dependencies that are not being used is even worst.

Since Handsontable seems to be helpful in term of performances, it might still be worth it to have those unnecessary dependencies. However, after this PR, we should try to find a way to get rid of them somehow (by keeping an eye for a fix in handsontable, or making a PR there, or even trying their hotbuilder)

r-kamath · 2016-05-12T15:05:43Z

@corneadoug I'm following up on handsontable repo. Will find some time to explore for a potential PR.

corneadoug · 2016-05-12T15:07:23Z

Merging if there is no more discussions

ghost · 2016-05-20T01:26:24Z

Great Job. I have a silly question, how do you debug your code in zeppelin, I mean , what IDE do you use and how you make breakpoint or sth like that ...

corneadoug · 2016-05-20T01:36:01Z

@yantaiv Better ask that in the mailing list, you would get more answers
But overall:
You can use any IDE you want, there is informations about dev mode in the README

gauravkumar37 · 2016-06-03T05:32:08Z

Thanks for the wonderful commit.
However, I faced one issue. I am not able to select any data in the table using mouse for copying it. (checked on FF & chrome latest versions)

…nse file. ### What is this PR for? <Jquery.floatThead> is no longer used. But the license document contains the appropriate information. Due to the following PR <jquery.floatThead> was removed. Resulting in a using < HandsonTable >. #858 ### What type of PR is it? Documentation ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-953 ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: CloverHearts <cloverheartsdev@gmail.com> Closes #960 from cloverhearts/ZEPPELIN-953 and squashes the following commits: d9ac1f1 [CloverHearts] Merge branch 'master' into ZEPPELIN-953 5cddba6 [CloverHearts] deleted jquery.floatThead at license file

corneadoug · 2016-06-09T03:47:28Z

@gauravkumar37 handled in #973

### What is this PR for? This PR fixes a few minor issues from the recent introduction of Handsontable for table rendering (#858): * Render up to 5 digits after decimal point instead of always rounding to integers * Allow visual selection of table cells (for copy) * Default to text renderer instead of numeric renderer ### What type of PR is it? Bug Fix, Improvement ### Todos ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-973 * https://issues.apache.org/jira/browse/ZEPPELIN-954 ### How should this be tested? Output some rows with floating point numbers and render them in a table. ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Hao Xia <hao@optimizely.com> Closes #973 from jasonxh/hao/render-table and squashes the following commits: a663833 [Hao Xia] Remove cell selection. Allow visually selecting table text. 7bc85b5 [Hao Xia] Table rendering improvements: * Render up to 5 digits after decimal point * Allow visual selection of table cells * Default to text renderer

ZEPPELIN-830 Improve table display to handle large data

c833f6e

fix selenium test failure

4e6beb8

felixcheung mentioned this pull request Apr 27, 2016

ZEPPELIN-672] Add Datatables instead of normal tables #761

Closed

1 task

Leemoonsoo reviewed Apr 28, 2016
View reviewed changes

ZEPPELIN-830 fix number formatting for table display

7bb508e

ZEPPELIN-830 License update

de6d134

ZEPPELIN-830 fix jshint error

b87245a

r-kamath force-pushed the ZEPPELIN-830 branch from ff10ca3 to b87245a Compare April 29, 2016 03:32

ZEPPELIN-830 fix test

b365e7f

ZEPPELIN-830 fix rendering without angular/html directives

a96c94a

r-kamath force-pushed the ZEPPELIN-830 branch from 44ef274 to a96c94a Compare April 29, 2016 12:35

ZEPPELIN-830 remove unused floatThead dep

adb8380

r-kamath added 2 commits April 30, 2016 09:24

ZEPPELIN-830 update licence doc

2e422ea

Merge branch 'master' of https://github.com/apache/incubator-zeppelin …

4fbb22b

…into ZEPPELIN-830

ZEPPELIN-830 fix wrong variable name

dd0ced7

r-kamath force-pushed the ZEPPELIN-830 branch from 1c6b5c0 to dd0ced7 Compare May 4, 2016 16:09

corneadoug mentioned this pull request May 9, 2016

Added CSV and Tableau export functionality #725

Closed

Merge branch 'master' of https://github.com/apache/incubator-zeppelin …

f13ba91

…into ZEPPELIN-830

asfgit closed this in 4eccfcd May 16, 2016

cloverhearts mentioned this pull request Jun 4, 2016

ZEPPELIN-953 ] Jquery.floatThead delete the information from the license file. #960

Closed

jasonxh mentioned this pull request Jun 7, 2016

[ZEPPELIN-973, ZEPPELIN-954] Table rendering improvements #973

Closed

ZEPPELIN-830 Improve table display to handle large data #858

ZEPPELIN-830 Improve table display to handle large data #858

Uh oh!

Conversation

r-kamath commented Apr 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

prabhjyotsingh commented Apr 26, 2016

Uh oh!

astroshim commented Apr 26, 2016

Uh oh!

felixcheung commented Apr 26, 2016

Uh oh!

r-kamath commented Apr 27, 2016

Uh oh!

felixcheung commented Apr 27, 2016

Uh oh!

r-kamath commented Apr 28, 2016

Uh oh!

Leemoonsoo Apr 28, 2016

Choose a reason for hiding this comment

Uh oh!

r-kamath Apr 28, 2016

Choose a reason for hiding this comment

Uh oh!

r-kamath Apr 29, 2016

Choose a reason for hiding this comment

Uh oh!

Leemoonsoo commented Apr 28, 2016

Uh oh!

r-kamath commented Apr 28, 2016

Uh oh!

r-kamath commented Apr 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Leemoonsoo commented Apr 30, 2016

Uh oh!

r-kamath commented May 2, 2016

Uh oh!

prabhjyotsingh commented May 4, 2016

Uh oh!

Leemoonsoo commented May 4, 2016

Uh oh!

r-kamath commented May 4, 2016

Uh oh!

Leemoonsoo commented May 4, 2016

Uh oh!

r-kamath commented May 4, 2016

Uh oh!

Leemoonsoo commented May 4, 2016

Uh oh!

felixcheung commented May 4, 2016

Uh oh!

corneadoug commented May 5, 2016

Uh oh!

r-kamath commented May 5, 2016

Uh oh!

prabhjyotsingh commented May 9, 2016

Uh oh!

r-kamath commented May 10, 2016

Uh oh!

prabhjyotsingh commented May 10, 2016

Uh oh!

corneadoug commented May 10, 2016

Uh oh!

r-kamath commented May 10, 2016

Uh oh!

corneadoug commented May 10, 2016

Uh oh!

r-kamath commented May 10, 2016

Uh oh!

prabhjyotsingh commented May 10, 2016

Uh oh!

r-kamath commented Apr 26, 2016 •

edited

Loading

r-kamath commented Apr 29, 2016 •

edited

Loading

corneadoug commented May 10, 2016 •

edited

Loading