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 2021/08/25 08:26:30 UTC
[GitHub] [echarts] benlaski opened a new pull request #15601: Add support for StreamGraph algorithm#14643
benlaski opened a new pull request #15601:
URL: https://github.com/apache/echarts/pull/15601
## Brief Information
This pull request is in the type of new feature to solve #14643 issue.
### Before:
ECharts currently supports the themeRiver algorithm for drawing area charts centered around the x-axis.
### After:
Set the default option to "symmetric"(the origin echarts rendering method) and add the wiggle algorithm as another rendering option according to the Stacked Graph by Lee Byron.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-918826965
CI is failed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] benlaski commented on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
benlaski commented on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-930752702
Have fixed the coding style issues and renamed ‘drawMode’ to ‘centerMode’.
Please let me know if there are further problems.
从 Windows 版邮件<https://go.microsoft.com/fwlink/?LinkId=550986>发送
发件人: Yi ***@***.***>
发送时间: 2021年9月30日 9:51
收件人: ***@***.***>
抄送: ***@***.***>; ***@***.***>
主题: Re: [apache/echarts] Add support for StreamGraph algorithm#14643 (#15601)
@pissang requested changes on this pull request.
In src/chart/themeRiver/themeRiverLayout.ts<https://github.com/apache/echarts/pull/15601#discussion_r718998608>:
> @@ -95,7 +96,16 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
});
const base = computeBaseline(layerPoints);
- const baseLine = base.y0;
+ var baseLine;
+ if (drawMode === 'symmetrical')
+ {
{ should not be in a newline.
In src/chart/themeRiver/themeRiverLayout.ts<https://github.com/apache/echarts/pull/15601#discussion_r718998699>:
> @@ -128,38 +138,45 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
+ function computeBaseline(data: number[][][]) {
Extra space here?
In src/chart/themeRiver/themeRiverLayout.ts<https://github.com/apache/echarts/pull/15601#discussion_r718999062>:
> for (let j = 0; j < layerNum; ++j) {
temp += data[j][i][1];
+ tempWiggle += (layerNum-(j+1)) * data[j][i][1];
Should be spaces on both sides of the operator
―
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://github.com/apache/echarts/pull/15601#pullrequestreview-767325522>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/APYJ4EL5MH5FNAJOBIEYUELUEO7DPANCNFSM5CYQ2NJA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r719081919
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -131,35 +138,42 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
function computeBaseline(data: number[][][]) {
const layerNum = data.length;
const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ const sums = [];//sums of each vertical row at a single time point
+ const sumsWiggle = [];
+ const y0Symmetric = [];
+ const y0Wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let tempWiggle = 0;
for (let j = 0; j < layerNum; ++j) {
temp += data[j][i][1];
+ tempWiggle += (layerNum - (j+1)) * data[j][i][1];
Review comment:
Still not spaced on '+' operator. Please check the code style carefully
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang edited a comment on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-930682983
The code looks good. But I think option `drawMode` can have another name. `drawMode` in echarts is more like `fill`, `stroke`, etc. We can use `centerMode` for example?
BTW it will be better if we can have screenshots for both mode in this pull request description
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r719081919
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -131,35 +138,42 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
function computeBaseline(data: number[][][]) {
const layerNum = data.length;
const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ const sums = [];//sums of each vertical row at a single time point
+ const sumsWiggle = [];
+ const y0Symmetric = [];
+ const y0Wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let tempWiggle = 0;
for (let j = 0; j < layerNum; ++j) {
temp += data[j][i][1];
+ tempWiggle += (layerNum - (j+1)) * data[j][i][1];
Review comment:
Still not spaced on '+' operator
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] quillblue commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
quillblue commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r717222040
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
+ const layerNum = data.length;//6
+ const pointNum = data[0].length;//21
+ const sums = [];//sums of each vertical row at a single time point
+ const sums1 = [];
+ const y0_symmetric = [];
+ const y0_wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let temp1 = 0;
for (let j = 0; j < layerNum; ++j) {
temp += data[j][i][1];
+ temp1 += (layerNum-(j+1))*data[j][i][1];
}
if (temp > max) {
max = temp;
}
sums.push(temp);
+ sums1.push(temp1)
}
- for (let k = 0; k < pointNum; ++k) {
- y0[k] = (max - sums[k]) / 2;
+
+ for (let k = 0 ; k < pointNum; ++k) {
+ y0_symmetric[k] = (max-sums[k])/2
Review comment:
should it be `(max - sums[k]) / 2` ? (according to code style requirement of community)
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
+ const layerNum = data.length;//6
+ const pointNum = data[0].length;//21
+ const sums = [];//sums of each vertical row at a single time point
+ const sums1 = [];
+ const y0_symmetric = [];
+ const y0_wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let temp1 = 0;
Review comment:
Considering optimize name `temp1` to indicate the meaning of this temp
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -95,7 +96,15 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
});
const base = computeBaseline(layerPoints);
- const baseLine = base.y0;
+ var baseLine;
+ if(drawMode == "symmetrical")
Review comment:
Coding style: [MUST] Place 1 space after `if` / `else` / `for` / `while` / `function` / `switch` / `do` / `try` / `catch` / `finally`.
Code smell: use `===` instead of `==` for strict requirement
##########
File path: src/chart/themeRiver/ThemeRiverSeries.ts
##########
@@ -69,6 +69,12 @@ export interface ThemeRiverSeriesOption extends SeriesOption<ThemeRiverStateOpti
* [date, value, name]
*/
data?: ThemerRiverDataItem[]
+
+ /**
+ * draw mode symmetrical or asymmetical
+ * default symmetrical
+ */
+ drawMode ?:"symmetrical"|"wiggle"
Review comment:
Coding Style: should it be `drawMode?: "symmetrical"|"wiggle"`?
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
+ const layerNum = data.length;//6
Review comment:
What did this comment mean?
##########
File path: test/themeRiver.html
##########
@@ -88,6 +88,7 @@
shadowColor: 'rgba(0, 0, 0, 0.8)'
}
},
+ drawMode:"wiggle",
Review comment:
`drawMode: 'wiggle'`
##########
File path: src/chart/themeRiver/ThemeRiverSeries.ts
##########
@@ -134,7 +140,6 @@ class ThemeRiverSeriesModel extends SeriesModel<ThemeRiverSeriesOption> {
const timeValue = layerData[k].dataList[j][0] + '';
timeValueKeys[timeValue] = k;
}
-
Review comment:
[Code Style] remaining empty line here (according to Code Style requirement of community)
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
Review comment:
Remove unused commented code
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-930682983
The code looks good. But I think option `drawMode` can have another name. `drawMode` in echarts is more like `fill`, `stroke`, etc. We can use `centerMode` for example?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] quillblue commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
quillblue commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r717222040
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
+ const layerNum = data.length;//6
+ const pointNum = data[0].length;//21
+ const sums = [];//sums of each vertical row at a single time point
+ const sums1 = [];
+ const y0_symmetric = [];
+ const y0_wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let temp1 = 0;
for (let j = 0; j < layerNum; ++j) {
temp += data[j][i][1];
+ temp1 += (layerNum-(j+1))*data[j][i][1];
}
if (temp > max) {
max = temp;
}
sums.push(temp);
+ sums1.push(temp1)
}
- for (let k = 0; k < pointNum; ++k) {
- y0[k] = (max - sums[k]) / 2;
+
+ for (let k = 0 ; k < pointNum; ++k) {
+ y0_symmetric[k] = (max-sums[k])/2
Review comment:
should it be `(max - sums[k]) / 2` ? (according to code style requirement of community)
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
+ const layerNum = data.length;//6
+ const pointNum = data[0].length;//21
+ const sums = [];//sums of each vertical row at a single time point
+ const sums1 = [];
+ const y0_symmetric = [];
+ const y0_wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let temp1 = 0;
Review comment:
Considering optimize name `temp1` to indicate the meaning of this temp
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -95,7 +96,15 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
});
const base = computeBaseline(layerPoints);
- const baseLine = base.y0;
+ var baseLine;
+ if(drawMode == "symmetrical")
Review comment:
Coding style: [MUST] Place 1 space after `if` / `else` / `for` / `while` / `function` / `switch` / `do` / `try` / `catch` / `finally`.
Code smell: use `===` instead of `==` for strict requirement
##########
File path: src/chart/themeRiver/ThemeRiverSeries.ts
##########
@@ -69,6 +69,12 @@ export interface ThemeRiverSeriesOption extends SeriesOption<ThemeRiverStateOpti
* [date, value, name]
*/
data?: ThemerRiverDataItem[]
+
+ /**
+ * draw mode symmetrical or asymmetical
+ * default symmetrical
+ */
+ drawMode ?:"symmetrical"|"wiggle"
Review comment:
Coding Style: should it be `drawMode?: "symmetrical"|"wiggle"`?
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
+ const layerNum = data.length;//6
Review comment:
What did this comment mean?
##########
File path: test/themeRiver.html
##########
@@ -88,6 +88,7 @@
shadowColor: 'rgba(0, 0, 0, 0.8)'
}
},
+ drawMode:"wiggle",
Review comment:
`drawMode: 'wiggle'`
##########
File path: src/chart/themeRiver/ThemeRiverSeries.ts
##########
@@ -134,7 +140,6 @@ class ThemeRiverSeriesModel extends SeriesModel<ThemeRiverSeriesOption> {
const timeValue = layerData[k].dataList[j][0] + '';
timeValueKeys[timeValue] = k;
}
-
Review comment:
[Code Style] remaining empty line here (according to Code Style requirement of community)
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +137,46 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
- const layerNum = data.length;
- const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ function computeBaseline(data: number[][][]) {
+ //console.log(data);
Review comment:
Remove unused commented code
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang edited a comment on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-930682983
The code looks good except some code style issues. But I think option `drawMode` can have another name. `drawMode` in echarts is more like `fill`, `stroke`, etc. We can use `centerMode` for example?
BTW it will be better if we can have screenshots for both mode in this pull request description
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r719081593
##########
File path: src/chart/themeRiver/ThemeRiverSeries.ts
##########
@@ -69,6 +69,11 @@ export interface ThemeRiverSeriesOption extends SeriesOption<ThemeRiverStateOpti
* [date, value, name]
*/
data?: ThemerRiverDataItem[]
+ /**
+ * center mode symmetrical or asymmetical
+ * default symmetrical
+ */
+ centerMode ?:'symmetrical'|'wiggle'
Review comment:
extra space here. run `npm run lint` before commit
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] echarts-bot[bot] commented on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-905293252
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/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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] quillblue commented on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
quillblue commented on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-918796415
LGTM on the whole, with following minor items to be considered:
- Coding format (unnecessary empty line)
- Not sure whether it is proper to use `List` instead of `SeriesData` here (need input from @pissang )
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r718998608
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -95,7 +96,16 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
});
const base = computeBaseline(layerPoints);
- const baseLine = base.y0;
+ var baseLine;
+ if (drawMode === 'symmetrical')
+ {
Review comment:
`{` should not be in a newline.
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +138,45 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
+ function computeBaseline(data: number[][][]) {
const layerNum = data.length;
const pointNum = data[0].length;
- const sums = [];
- const y0 = [];
+ const sums = [];//sums of each vertical row at a single time point
+ const sumsWiggle = [];
+ const y0Symmetric = [];
+ const y0Wiggle = [];
let max = 0;
for (let i = 0; i < pointNum; ++i) {
let temp = 0;
+ let tempWiggle = 0;
for (let j = 0; j < layerNum; ++j) {
temp += data[j][i][1];
+ tempWiggle += (layerNum-(j+1)) * data[j][i][1];
Review comment:
Should be spaces on both sides of the operator
##########
File path: src/chart/themeRiver/themeRiverLayout.ts
##########
@@ -128,38 +138,45 @@ function doThemeRiverLayout(data: SeriesData<ThemeRiverSeriesModel>, seriesModel
*
* @param data the points in each layer
*/
-function computeBaseline(data: number[][][]) {
+ function computeBaseline(data: number[][][]) {
Review comment:
Extra space here?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] pissang commented on a change in pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#discussion_r707931734
##########
File path: src/chart/themeRiver/ThemeRiverSeries.ts
##########
@@ -18,9 +18,9 @@
*/
import SeriesModel from '../../model/Series';
-import prepareSeriesDataSchema from '../../data/helper/createDimensions';
+import createDimensions from '../../data/helper/createDimensions';
import {getDimensionTypeByAxis} from '../../data/helper/dimensionHelper';
-import SeriesData from '../../data/SeriesData';
+import List from '../../data/List';
Review comment:
Should update the code to the latest
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] benlaski edited a comment on pull request #15601: Add support for StreamGraph algorithm#14643
Posted by GitBox <gi...@apache.org>.
benlaski edited a comment on pull request #15601:
URL: https://github.com/apache/echarts/pull/15601#issuecomment-930752702
Have fixed the coding style issues and renamed ‘drawMode’ to ‘centerMode’.
Please let me know if there are further problems
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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