You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2020/06/18 03:22:13 UTC

[GitHub] [incubator-echarts] wangyv opened a new pull request #12821: feat(sankey): provide layout with unequal input and output paths

wangyv opened a new pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821


   <!-- Please fill in the following information to help us review your PR more efficiently. -->
   
   ## Brief Information
   
   This pull request is in the type of:
   
   - [ ] bug fixing
   - [ ] new feature
   - [ ] others
   
   
   
   ### What does this PR do?
   
   <!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
   
   
   
   ### Fixed issues
   
   <!--
   - #xxxx: ...
   -->
   
   
   ## Details
   
   ### Before: What was the problem?
   
   <!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   
   
   
   ### After: How is it fixed in this PR?
   
   <!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   
   
   
   ## Usage
   
   ### Are there any API changes?
   
   - [ ] The API has been changed.
   
   <!-- LIST THE API CHANGES HERE -->
   
   
   
   ### Related test cases or examples to use the new APIs
   
   NA.
   
   
   
   ## Others
   
   ### Merging options
   
   - [ ] Please squash the commits into a single one when merge.
   
   ### Other information
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] susiwen8 commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645753385


   @wangyv Just submit source file you have changed


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] wangyv commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
wangyv commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645755980


   > @wangyv Just submit source file you have changed
   
   done @susiwen8 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] Ovilia commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
Ovilia commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r442010563



##########
File path: src/chart/sankey/sankeyLayout.js
##########
@@ -28,6 +28,8 @@ export default function (ecModel, api, payload) {
         var nodeWidth = seriesModel.get('nodeWidth');
         var nodeGap = seriesModel.get('nodeGap');
 
+        var mode = seriesModel.get('mode') || 'normal';

Review comment:
       You should probably set default mode in `SankeySeries.js` rather than using `|| 'normal'`.

##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,60 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+  var sum = 0;

Review comment:
       Please use 4 spaces for indent.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r441963199



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -132,6 +154,7 @@ export default echarts.extendChartView({
         var width = layoutInfo.width;
         // view height
         var height = layoutInfo.height;
+        var mode = seriesModel.option.mode || 'normal';

Review comment:
       It's suggested to use `seriesModel.get('mode')` instead of `seriesModel.option.mode`.
   ```suggestion
           var mode = seriesModel.get('mode') || 'normal';
   ```

##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -183,14 +219,15 @@ export default echarts.extendChartView({
             }
             else {
                 x1 = (dragX1 != null ? dragX1 * width : n1Layout.x) + n1Layout.dx;
-                y1 = (dragY1 != null ? dragY1 * height : n1Layout.y) + edgeLayout.sy;
+                y1 = (dragY1 != null ? dragY1 * height : n1Layout.y / n1lengthin) + edgeLayout.sy / n1lengthout;
                 x2 = dragX2 != null ? dragX2 * width : n2Layout.x;
-                y2 = (dragY2 != null ? dragY2 * height : n2Layout.y) + edgeLayout.ty;
+                y2 = (dragY2 != null ? dragY2 * height : n2Layout.y / n2lengthout) + edgeLayout.ty / n2lengthin;
                 cpx1 = x1 * (1 - curvature) + x2 * curvature;
                 cpy1 = y1;
                 cpx2 = x1 * curvature + x2 * (1 - curvature);
                 cpy2 = y2;
             }
+            // debugger

Review comment:
       This debugger should be removed.

##########
File path: src/chart/sankey/sankeyLayout.js
##########
@@ -28,6 +28,8 @@ export default function (ecModel, api, payload) {
         var nodeWidth = seriesModel.get('nodeWidth');
         var nodeGap = seriesModel.get('nodeGap');
 
+        var mode = seriesModel.option.mode || 'normal';

Review comment:
       It's suggested to use `seriesModel.get('mode')` instead of `seriesModel.option.mode`.
   ```suggestion
           var mode = seriesModel.get('mode') || 'normal';
   ```

##########
File path: src/chart/sankey/sankeyLayout.js
##########
@@ -82,13 +84,17 @@ function layoutSankey(nodes, edges, nodeWidth, nodeGap, width, height, iteration
  * Compute the value of each node by summing the associated edge's value
  *
  * @param {module:echarts/data/Graph~Node} nodes  node of sankey view
+ * @param {string} mode  mode of sankey view
  */
-function computeNodeValues(nodes) {
+function computeNodeValues(nodes, mode) {
     zrUtil.each(nodes, function (node) {
         var value1 = sum(node.outEdges, getEdgeValue);
         var value2 = sum(node.inEdges, getEdgeValue);
         var nodeRawValue = node.getValue() || 0;
         var value = Math.max(value1, value2, nodeRawValue);
+        if (mode === 'uneven') {
+          value = nodeRawValue || value;
+        }
         node.setLayout({value: value}, true);
     });

Review comment:
       The judging logic of `mode === 'uneven'` is suggested to move to the place before `zrUtil.each`.
   ```suggestion
       var isUnevenMode = mode === 'uneven';
       zrUtil.each(nodes, function (node) {
           var value1 = sum(node.outEdges, getEdgeValue);
           var value2 = sum(node.inEdges, getEdgeValue);
           var nodeRawValue = node.getValue() || 0;
           var value = Math.max(value1, value2, nodeRawValue);
           if (isUnevenMode) {
             value = nodeRawValue || value;
           }
           node.setLayout({value: value}, true);
       });
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] Ovilia commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
Ovilia commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r444616890



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,60 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+    var sum = 0;
+    var len = array.length;
+    var i = -1;
+    while (++i < len) {
+        var value = +cb.call(array, array[i], orient);
+        if (!isNaN(value)) {
+            sum += value;
+        }
+    }

Review comment:
       Yes, we don't use ES6 features. Please use `zrUtil.forEach` instead.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] wangyv commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
wangyv commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r442018795



##########
File path: src/chart/sankey/sankeyLayout.js
##########
@@ -28,6 +28,8 @@ export default function (ecModel, api, payload) {
         var nodeWidth = seriesModel.get('nodeWidth');
         var nodeGap = seriesModel.get('nodeGap');
 
+        var mode = seriesModel.get('mode') || 'normal';

Review comment:
       done @Ovilia 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r444575312



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,60 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+    var sum = 0;
+    var len = array.length;
+    var i = -1;
+    while (++i < len) {
+        var value = +cb.call(array, array[i], orient);
+        if (!isNaN(value)) {
+            sum += value;
+        }
+    }

Review comment:
       I'm afraid we can not use `for in` in array traverse, for one thing, it's limited by [ECharts coding-standard](https://echarts.apache.org/en/coding-standard.html#others)(and just ES3 is allowed), and for another, it's reasonable why we don't use as far as possible.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] susiwen8 commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645752982


   Sorry, I didn't make myself clear, you shouldn't submit dist/, not just remove it 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645747200


   Thanks for your contribution!
   The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/incubator-echarts/wiki/How-to-make-a-pull-request).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] wangyv commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
wangyv commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r442018740



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,60 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+  var sum = 0;

Review comment:
       done @Ovilia 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] wangyv commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
wangyv commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r446608109



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,60 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+    var sum = 0;
+    var len = array.length;
+    var i = -1;
+    while (++i < len) {
+        var value = +cb.call(array, array[i], orient);
+        if (!isNaN(value)) {
+            sum += value;
+        }
+    }

Review comment:
       @Ovilia done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] wangyv commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
wangyv commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645752725


   > Please remove dist/ files
   
   done @susiwen8 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] plainheart commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645808212


   @wangyv Thank you! We will review this feature later.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] chfw commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
chfw commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r444528338



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,60 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+    var sum = 0;
+    var len = array.length;
+    var i = -1;
+    while (++i < len) {
+        var value = +cb.call(array, array[i], orient);
+        if (!isNaN(value)) {
+            sum += value;
+        }
+    }

Review comment:
       Please try this es6 for..in looping syntax:
   
   var a =[ 3, 2, 1]
   for(index in a){
       // do something with it.
       console.log(index);
   }




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] wangyv commented on pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
wangyv commented on pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#issuecomment-645802796


   > If possible, could you please provide some screenshots for this PR to make this new feature more specific?
   
   done @plainheart 
   ![screenshots](https://sf3-ttcdn-tos.pstatp.com/img/motor-img/a61293ac974f8e82863722436434bc21~noop.jpg)
   
   ![screenshots2](https://sf1-ttcdn-tos.pstatp.com/img/motor-img/26938ce912b41931af63ea201c9edbfa~noop.jpg)
   
   ![screenshots3](https://sf3-ttcdn-tos.pstatp.com/img/motor-img/629d5ab3dc1a062062c90238e26cf896~noop.jpg)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] chfw commented on a change in pull request #12821: feat(sankey): provide layout with unequal input and output paths

Posted by GitBox <gi...@apache.org>.
chfw commented on a change in pull request #12821:
URL: https://github.com/apache/incubator-echarts/pull/12821#discussion_r548090387



##########
File path: src/chart/sankey/SankeyView.js
##########
@@ -61,38 +61,58 @@ function fadeInItem(item, opacityPath) {
     el.highlight && el.highlight();
 }
 
+function sum(array, cb, orient) {
+    var sum = 0;
+    zrUtil.each(array, function (data) {
+      var value = +cb.call(array, data, orient);
+        if (!isNaN(value)) {
+            sum += value;
+        }
+    });
+    return sum;
+}
+
+function getEdgeValue(edge) {
+    return edge.getValue();
+}
+
 var SankeyShape = graphic.extendShape({
     shape: {
         x1: 0, y1: 0,
         x2: 0, y2: 0,
         cpx1: 0, cpy1: 0,
         cpx2: 0, cpy2: 0,
         extent: 0,
-        orient: ''
+        extent1: 0,
+        extent2: 0,
+        orient: '',
+        mode: ''
     },

Review comment:
       a human readable variable name would improve its maintenance effort greatly.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org