[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2098 Tremendous work @1ambda ! LGTM and merge to master if no further discussions. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 Removed `WIP` from PR title. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098  @AhyoungRyu I forgot as well to modify `index.md` ð. I'v just updated. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda Looks great! So sorry I forgot to mention that there is another place to be updated regarding [this change](https://github.com/apache/zeppelin/pull/2098#issuecomment-291390619). https://cloud.githubusercontent.com/assets/10060731/24690393/7e4612ec-1a07-11e7-8d70-34554826a27a.png; width="600px"> Could you update [`docs/index.html`](https://github.com/apache/zeppelin/blob/master/docs/index.md#more) as well? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 @AhyoungRyu resolved all comments. Thanks for detailed 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 @AhyoungRyu Updated docs as well.  --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 @AhyoungRyu 1. Hide other buttons when panel is minified  2. Added doc to `docs/development/writingzeppelinvisualization_transformation.md`. It looks like  3. Regarding to popup, i think we can skip it because if parameter is applied or reset, the chart will change according to the options. The chart will remain same only if parameter is not changed at all and user can recognize when he/she does't change parameter. @Leemoonsoo 4. I added many tests to `advanced-transformation-util.test.js`. It covers most cases and many functions. Here is the file. - https://github.com/1ambda/zeppelin/blob/6a0559b18542c60b1211095f7583a71e5fb0c136/zeppelin-web/src/app/tabledata/advanced-transformation-util.test.js --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda Appreciate for such detailed explanation! Regarding `no-group`, I can assume the other ppl can be confused like me. So it'll be better to add some description for that to the documentation. (you already wrote all needed contents in your comments :D) FYI I suggested to separate Helium related docs page from "Contribute" section in the dropdown menu and create new section "Helium Framework" in #2202(Helium is worth to have its own kk). So your new docs page "Writing Zeppelin Visualization: Transformation(or another name?)" can be put under this section. And maybe more self-descriptive name would be better instead of just `Transformation`(hard to recognize at a glance what kind of transformation is). Something like `Writing Zeppelin Visualization: Advanced Chart Configuration`.. But I don't wanna hang this PR because of docs. Surely you can handle it in another 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 @AhyoungRyu Thanks for detailed review. 1. It would be nice to have some popup or feedback when conf is updated. 2. Let me hide them when panel is minimized (e.g the chart panel, the parameter panel) Regarding to `3. no-group field`, it's quite hard to say in just few lines. So let me describe more. (A) Some charts require aggregation. (= groupBy in SQL) (B) But some charts might not need aggregation. Assume that it's already grouped by SQL In ultimate-line-chart, there are sub charts called `line`, `dashed`, `step` and `no-group`. - `line`, `dashed`,` step` uses aggregation like zeppelin's existing charts. (similar to `PivotTransformation`) - But `no-group` doesn't use aggregation. (similar to `ColumnTransformation`) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda I feel so sorry for my late response. Tested this patch \w your Helium package: zeppelin-ultimate-line-chart. Here are some thought and question(?) in my head. 1. "Save" & "Delete" button on top of the setting table - It'll be better there are some feedback after clicking these buttons like "Changes are successfully saved" or "Changes will be discarded". How about small `ngtoast` or bootstrap dialog?  Probably ppl can click the trash can button by mistake. Then they'll lose the setting. To prevent this, we can provide some alert for this. - I think we can hide "Save" & "Delete" button when the setting table is folded. A good example is in our interpreter setting page. As you know, users only can save/remove the setting when they click "edit" button. So how about activating those disket & trash btns only when the setting table is expanded? 2. no-group field - What is difference between `no-group` and just letting `group` field as blank? Is there special reason for creating `no-group` field? (It's question actually :D) - `xAxis` and `yAxis` field size are different in `no-group` section. https://cloud.githubusercontent.com/assets/10060731/24440240/0ae00fc0-148e-11e7-999b-554af8d39f72.png;> And again, this is really cool! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 Let me add docs and unit tests. I guess it will take few days. Then i will comment. Thanks @Leemoonsoo --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda Great work!!! Looks great to me! I think it's worth to move usages and examples in this PR description under `/docs`. Where do you think this information can be placed? We didn't have any unittest for pivot transformation. So it was hard to improve / change without any side effects. So having some basic unittest for advanced transformation will help other people add and improve the feature much faster. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda Looks awesome! Let me test it and i'll get back to you soon. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 updated Please check https://github.com/1ambda/zeppelin-ultimate-line-chart as well. Thanks! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda Great work! Let me test this interesting feature in this weekend. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 I'v just updated the PR description. Please help review this @AhyoungRyu :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 CI passed. I will update PR description --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2098 @AhyoungRyu Thanks! I didn't attach screenshots for examples which use `aggregator: false,`. I will prepare in few days. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2098: [ZEPPELIN-2217] AdvancedTransformation for Visualizati...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/2098 @1ambda Cool! Let me test this and I will get back you soon. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
