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 2022/08/26 12:57:54 UTC
[GitHub] [echarts] konrad-amtenbrink opened a new pull request, #17579: Feat/box plot labels
konrad-amtenbrink opened a new pull request, #17579:
URL: https://github.com/apache/echarts/pull/17579
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
Box plot labels for all displayed values.
This pull request is in the type of:
- [ ] bug fixing
- [x] new feature
- [ ] others
### What does this PR do?
<!-- USE ONE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
This PR implements labels for all visible/plotted values of the box plot (this also includes the whiskers).
## Details
The box plot now utilises the basics of the label option (i.e. `show`, `offset` and `formatter`) to display labels for the values of each box of the box plot.
As the multiple labels are not supported for single series items - the `setBoxLabels` method was added to the `BoxView`. In order to custom calculate the positions and add the labels to the plot.
## Document Info
One of the following should be checked.
- [x] This PR doesn't relate to document changes
- [ ] The document should be updated later
- [ ] The document changes have been made in apache/echarts-doc#xxx
--
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] Ovilia commented on a diff in pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
Ovilia commented on code in PR #17579:
URL: https://github.com/apache/echarts/pull/17579#discussion_r1050426707
##########
test/boxplot-labels.html:
##########
@@ -0,0 +1,256 @@
+<!DOCTYPE html>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+
+<html>
+ <head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
+ <script src="lib/simpleRequire.js"></script>
+ <script src="lib/config.js"></script>
+ <script src="lib/jquery.min.js"></script>
+ <script src="lib/facePrint.js"></script>
+ <script src="lib/testHelper.js"></script>
+ <!-- <script src="ut/lib/canteen.js"></script> -->
+ <link rel="stylesheet" href="lib/reset.css" />
+ </head>
+ <body>
+ <style>
+ </style>
+ <div id="main"></div>
+ <script>
+ /**
+ * @see <https://en.wikipedia.org/wiki/Michelson%E2%80%93Morley_experiment>
+ * @see <http://bl.ocks.org/mbostock/4061502>
+ */
+ var chart;
+ var data;
+ var mean;
+
+ require([
+ 'echarts',
+ 'data/Michelson-Morley.json.js'
+ ], function (echarts, rawData) {
+ var env = echarts.env;
+
+ chart = echarts.init(document.getElementById('main'));
+
+ update('horizontal');
+
+ initControlPanel(env);
+
+ function update(layout) {
+
+ // mean = calculateMean(rawData);
+
+ var categoryAxis = {
+ type: 'category',
+ boundaryGap: true,
+ nameGap: 30,
+ splitArea: {
+ show: false
+ },
+ splitLine: {
+ show: false
+ }
+ };
+ var valueAxis = {
+ type: 'value',
+ name: 'km/s minus 299,000',
+ splitArea: {
+ show: true
+ }
+ };
+
+ chart.setOption({
+ aria: {
+ show: true
+ },
+ dataset: [{
+ source: rawData
+ }, {
+ transform: {
+ type: 'boxplot',
+ config: { itemNameFormatter: 'expr {value}' }
+ }
+ }, {
+ fromDatasetIndex: 1,
+ fromTransformResult: 1
+ }],
+ title: [
+ {
+ text: 'Michelson-Morley Experiment',
+ left: 'center'
+ },
+ {
+ text: 'upper: Q3 + 1.5 * IRQ \nlower: Q1 - 1.5 * IRQ',
+ borderColor: '#999',
+ borderWidth: 1,
+ textStyle: {
+ fontSize: 14
+ },
+ left: '10%',
+ top: '90%'
+ }
+ ],
+ legend: {
+ left: 'right'
+ },
+ tooltip: {
+ trigger: 'item',
+ axisPointer: {
+ type: 'shadow'
+ }
+ },
+ grid: {
+ left: '10%',
+ right: '10%',
+ bottom: '15%'
+ },
+ xAxis: layout === 'horizontal' ? categoryAxis : valueAxis,
+ yAxis: layout === 'vertical' ? categoryAxis : valueAxis,
+ series: [
+ {
+ name: 'boxplot',
+ type: 'boxplot',
+ datasetIndex: 1,
+
+ markPoint: {
+ data: [
+ {
+ name: '某个坐标',
+ coord: [2, 300]
+ },
+ {
+ name: '某个屏幕坐标',
+ x: 100,
+ y: 200,
+ label: {
+ normal: {
+ show: false,
+ formatter: 'asdf'
+ },
+ emphasis: {
+ show: true,
+ position: 'top',
+ formatter: 'zxcv'
+ }
+ }
+ },
+ {
+ name: 'max value (default)',
+ type: 'max'
+ },
+ {
+ name: 'min value (default)',
+ type: 'min'
+ },
+ {
+ name: 'max value (dim:Q1)',
+ type: 'max',
+ valueDim: 'Q1'
+ },
+ {
+ name: 'average value (dim:Q1)',
+ type: 'average',
+ valueDim: 'Q1'
+ }
+ ]
+ },
+
+ markLine: {
+ data: [
+ [
+ {name: '两个坐标之间的标线', coord: [1, 240]},
+ {coord: [2, 260]}
+ ],
+ [
+ {name: '两个屏幕坐标之间的标线', x: 50, y: 60},
+ {x: 70, y: 90}
+ ],
+ [
+ {name: 'max - min', type: 'max'},
+ {type: 'min'}
+ ],
+ {
+ name: 'min line',
+ type: 'min'
+ },
+ {
+ name: 'max line on dim:Q3',
+ type: 'max',
+ valueDim: 'Q3'
+ }
+ ]
+ },
+ label: {
+ show: true,
+ formatter: function (param) {
Review Comment:
If `formatter` is not provided here, there will be an error. You should not assert it to be defined.
##########
src/chart/boxplot/BoxplotView.ts:
##########
@@ -180,11 +184,19 @@ function updateNormalBoxData(
el.useStyle(data.getItemVisual(dataIndex, 'style'));
el.style.strokeNoScale = true;
-
el.z2 = 100;
const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex);
const emphasisModel = itemModel.getModel('emphasis');
+ const labelModel = seriesModel.get('label');
+
+ if (labelModel.show) {
+ const formattedLabels =
+ ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[])
+ .splice(1).map((value: number) => labelModel.formatter(value).toString());
Review Comment:
This is not the correct way to format labels. If `formatter` is not defined, this should go wrong. You should probably do like [sunburst](https://github.com/apache/echarts/blob/master/src/chart/sunburst/SunburstPiece.ts#L189).
##########
src/label/labelStyle.ts:
##########
@@ -292,6 +293,66 @@ function setLabelStyle<TLabelDataIndex>(
}
export { setLabelStyle };
+/**
+ * Set and create specific labels for box plot.
+ *
+ * This method should only be used for this case,
+ * as it manually creates multiple alternating labels
+ * specifically for the box plot.
+ */
+export function setBoxLabels(boxItemPositions: number[][], formattedLabels: Array<string>,
+ labelOption: any, group: ViewRootGroup) {
+ // get sorted and unique y positions of box items
+ const yPositions = boxItemPositions.map((pos: number[]) => pos[1]);
+ const uniqueYPositions: number[] = [];
+ yPositions.forEach(position => {
+ if (!uniqueYPositions.includes(position)) {
+ uniqueYPositions.push(position);
+ }
+ });
+ uniqueYPositions.sort(function (a: number, b: number) {
+ return b - a;
+ });
+
+ boxItemPositions.sort(function (a: number[], b: number[]) {
+ return a[0] - b[0];
+ });
+
+ // get alternating y positions for labels to avoid overlap
+ const uniqueAlternatingPositions = uniqueYPositions.map((posY: number, ind: number) => {
+ const matchingPositions = boxItemPositions.filter((orgPos: number[]) => orgPos[1] === posY);
+ const index = (ind % 2 === 0) ? 0 : matchingPositions.length - 1;
+ return matchingPositions[index];
+ });
+
+ // create labels and add them to their respective positions
+ formattedLabels.forEach((labelText: string, ind: number) => {
+ if (labelOption.show === 'iqr' && (ind === 0 || ind === 4)) {
+ return;
+ }
+ if (labelOption.show === 'median' && (ind !== 2)) {
+ return;
+ }
+ if (labelOption.show === 'whiskers' && (ind !== 0 && ind !== 4)) {
+ return;
+ }
+ const label = new graphic.Text({
+ style: {
+ text: labelText
+ },
+ z2: 1000
+ });
+ const defaultOffset = 5;
+ const defaultXOffset = (ind % 2) === 0 ? (-(label.getBoundingRect().width + defaultOffset)) : defaultOffset;
Review Comment:
As I said before, we should provide a way to let developers decide where to display the label to the left or right. The `label.align` can be changed into `'left' | 'right' | 'center' | { iqr: 'left' | 'right' | 'center' , ... }`.
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1240428013
@Ovilia I would vote to not add an option to display for example all labels on one side because they most likely will overlap and I do not see a use case for that (if there is someone with that specific use case - we can always open up a follow-up) - I can however add an option to specify which labels should be displayed
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1228459011
I am happy to move the `setBoxLabel` logic to a different file or even abstract it further - I was just not sure where it would be best suitable.
--
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] Ovilia commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
Ovilia commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1240185778
There are five labels (numbers) for each boxplot item, three of which is on the left side and two right. But some developers may have different requirement like only displaying the middle value or display all values on the left. How can that be 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.
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] DanielBogenrieder commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
DanielBogenrieder commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1346082301
Hey,
sorry to raise this again, but this would help us a lot. Are there any news on this?
--
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] Ovilia commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
Ovilia commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1235141729
Can you provide an image to illustrate what this PR is about?
--
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] DanielBogenrieder commented on a diff in pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
DanielBogenrieder commented on code in PR #17579:
URL: https://github.com/apache/echarts/pull/17579#discussion_r1007760902
##########
src/chart/boxplot/BoxplotView.ts:
##########
@@ -180,17 +183,76 @@ function updateNormalBoxData(
el.useStyle(data.getItemVisual(dataIndex, 'style'));
el.style.strokeNoScale = true;
-
el.z2 = 100;
const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex);
const emphasisModel = itemModel.getModel('emphasis');
+ if (seriesModel.option.label.show) {
+ const formattedLabels =
+ ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[])
+ .splice(1).map((value: number) => seriesModel.option.label.formatter(value).toString());
Review Comment:
@Ovilia any news on this? Is the proposed solution good enough?
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1240672946
@Ovilia the user now has the option to set ```label.show``` to either ```all```, ```iqr```, ```median``` or ```whiskers``` to change what labels are displayed.
--
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 #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1228456054
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1235163773
@Ovilia Sure - I added it to the 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] DanielBogenrieder commented on pull request #17579: Feat/box plot labels
Posted by "DanielBogenrieder (via GitHub)" <gi...@apache.org>.
DanielBogenrieder commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1453120772
Hey @Ovilia,
we still have some open questions to your comments. If you could clarify we could finish up the PR and get this merged?
Greetings and thanks,
Daniel
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1239274471
What do you mean by alignment of the labels?
--
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
Re: [PR] Feat/box plot labels [echarts]
Posted by "DanielBogenrieder (via GitHub)" <gi...@apache.org>.
DanielBogenrieder commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1991634134
Any news on this? Anything we can do to get it merged?
--
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] Ovilia commented on a diff in pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
Ovilia commented on code in PR #17579:
URL: https://github.com/apache/echarts/pull/17579#discussion_r974948463
##########
src/chart/boxplot/BoxplotView.ts:
##########
@@ -180,17 +183,76 @@ function updateNormalBoxData(
el.useStyle(data.getItemVisual(dataIndex, 'style'));
el.style.strokeNoScale = true;
-
el.z2 = 100;
const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex);
const emphasisModel = itemModel.getModel('emphasis');
+ if (seriesModel.option.label.show) {
Review Comment:
We don't access the attributes using `seriesModel.option`. Please use `const labelModel = seriesModel.get('label'); const show = labelModel.get('show');` instead.
Please also change `seriesModel.option` in other places.
##########
src/chart/boxplot/BoxplotView.ts:
##########
@@ -180,17 +183,76 @@ function updateNormalBoxData(
el.useStyle(data.getItemVisual(dataIndex, 'style'));
el.style.strokeNoScale = true;
-
el.z2 = 100;
const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex);
const emphasisModel = itemModel.getModel('emphasis');
+ if (seriesModel.option.label.show) {
+ const formattedLabels =
+ ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[])
+ .splice(1).map((value: number) => seriesModel.option.label.formatter(value).toString());
Review Comment:
You should probably not create a `Text` element and do all the stuffs by yourself. Instead, `setLabelStyle` should be called. See how this is done with [line series](https://github.com/apache/echarts/blob/master/src/chart/helper/Line.ts#L275-L289). But in boxplot's case, there are multiple labels so you need to figure out how this should be done, but generally speaking, `setLabelStyle` should be called most likely.
--
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] Ovilia commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
Ovilia commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1354309762
@DanielBogenrieder Sorry for late reply. I think this PR still need modification before merging.
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by "konrad-amtenbrink (via GitHub)" <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1511428940
Hey @Ovilia,
I left some questions on your comments because I am not sure if I understand them correctly - are there any updates on this?
Greetings and thanks,
Konrad
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by "konrad-amtenbrink (via GitHub)" <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1557228206
Hey @Ovilia,
are there any updates on this PR?
Thanks,
Konrad
--
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] konrad-amtenbrink commented on a diff in pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on code in PR #17579:
URL: https://github.com/apache/echarts/pull/17579#discussion_r990011936
##########
src/chart/boxplot/BoxplotView.ts:
##########
@@ -180,17 +183,76 @@ function updateNormalBoxData(
el.useStyle(data.getItemVisual(dataIndex, 'style'));
el.style.strokeNoScale = true;
-
el.z2 = 100;
const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex);
const emphasisModel = itemModel.getModel('emphasis');
+ if (seriesModel.option.label.show) {
+ const formattedLabels =
+ ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[])
+ .splice(1).map((value: number) => seriesModel.option.label.formatter(value).toString());
Review Comment:
I would not add it to `setLabelStyle` as it is super specific code which only applies to the bar chart, so it doesn't really make sense in my opinion to add it a general util method. However, I agree that it should live in the `labelStyle` util. So I would vote to move it there and refactor it so it does not create a Text element manually. What do you think?
--
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] konrad-amtenbrink commented on pull request #17579: Feat/box plot labels
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on PR #17579:
URL: https://github.com/apache/echarts/pull/17579#issuecomment-1249284269
Hey @Ovilia are there any updates on this - do you need further changes from my side? Thank you very much :)
--
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] konrad-amtenbrink commented on a diff in pull request #17579: Feat/box plot labels
Posted by "konrad-amtenbrink (via GitHub)" <gi...@apache.org>.
konrad-amtenbrink commented on code in PR #17579:
URL: https://github.com/apache/echarts/pull/17579#discussion_r1088992426
##########
src/label/labelStyle.ts:
##########
@@ -292,6 +293,66 @@ function setLabelStyle<TLabelDataIndex>(
}
export { setLabelStyle };
+/**
+ * Set and create specific labels for box plot.
+ *
+ * This method should only be used for this case,
+ * as it manually creates multiple alternating labels
+ * specifically for the box plot.
+ */
+export function setBoxLabels(boxItemPositions: number[][], formattedLabels: Array<string>,
+ labelOption: any, group: ViewRootGroup) {
+ // get sorted and unique y positions of box items
+ const yPositions = boxItemPositions.map((pos: number[]) => pos[1]);
+ const uniqueYPositions: number[] = [];
+ yPositions.forEach(position => {
+ if (!uniqueYPositions.includes(position)) {
+ uniqueYPositions.push(position);
+ }
+ });
+ uniqueYPositions.sort(function (a: number, b: number) {
+ return b - a;
+ });
+
+ boxItemPositions.sort(function (a: number[], b: number[]) {
+ return a[0] - b[0];
+ });
+
+ // get alternating y positions for labels to avoid overlap
+ const uniqueAlternatingPositions = uniqueYPositions.map((posY: number, ind: number) => {
+ const matchingPositions = boxItemPositions.filter((orgPos: number[]) => orgPos[1] === posY);
+ const index = (ind % 2 === 0) ? 0 : matchingPositions.length - 1;
+ return matchingPositions[index];
+ });
+
+ // create labels and add them to their respective positions
+ formattedLabels.forEach((labelText: string, ind: number) => {
+ if (labelOption.show === 'iqr' && (ind === 0 || ind === 4)) {
+ return;
+ }
+ if (labelOption.show === 'median' && (ind !== 2)) {
+ return;
+ }
+ if (labelOption.show === 'whiskers' && (ind !== 0 && ind !== 4)) {
+ return;
+ }
+ const label = new graphic.Text({
+ style: {
+ text: labelText
+ },
+ z2: 1000
+ });
+ const defaultOffset = 5;
+ const defaultXOffset = (ind % 2) === 0 ? (-(label.getBoundingRect().width + defaultOffset)) : defaultOffset;
Review Comment:
@Ovilia Sorry, maybe I am missing something here as well but there is no single label. We have five labels, right? I still don't get how someone could choose the align for the five in a way that makes sense - e.g. all labels on one side - even though they might overlap?
--
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] konrad-amtenbrink commented on a diff in pull request #17579: Feat/box plot labels
Posted by "konrad-amtenbrink (via GitHub)" <gi...@apache.org>.
konrad-amtenbrink commented on code in PR #17579:
URL: https://github.com/apache/echarts/pull/17579#discussion_r1088989421
##########
src/chart/boxplot/BoxplotView.ts:
##########
@@ -180,11 +184,19 @@ function updateNormalBoxData(
el.useStyle(data.getItemVisual(dataIndex, 'style'));
el.style.strokeNoScale = true;
-
el.z2 = 100;
const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex);
const emphasisModel = itemModel.getModel('emphasis');
+ const labelModel = seriesModel.get('label');
+
+ if (labelModel.show) {
+ const formattedLabels =
+ ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[])
+ .splice(1).map((value: number) => labelModel.formatter(value).toString());
Review Comment:
@Ovilia I am not sure if I understood correctly - `getFormattedLabel` is defined to return a `string` but somehow returns an `object` when being called with `dataIndex`.
I think I am missing something here, could you clarify how you would implement the formatting of the labels?
Thanks
--
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