korbit-ai[bot] commented on code in PR #34547:
URL: https://github.com/apache/superset/pull/34547#discussion_r2253269216
##########
superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts:
##########
@@ -143,6 +145,73 @@ export default function transformProps(
DEFAULT_ECHARTS_BOUNDS[1];
}
+ // Extract unique values for each axis
+ const xValues = Array.from(new Set(data.map(row => row[xAxisLabel])));
+ const yValues = Array.from(new Set(data.map(row => row[yAxisLabel])));
+
+ // Sort axis values based on configuration
+ const sortAxisValues = (
+ values: any[],
+ sortConfig: string | undefined,
+ axisLabel: string,
+ ) => {
+ if (!sortConfig) {
+ return values;
+ }
+
+ const isMetricSort = sortConfig.includes('value');
+ const isAscending = sortConfig.includes('asc');
+
+ if (isMetricSort) {
+ // Create a map of axis value to metric sum for sorting by metric
+ const metricSums: Record<string, number> = {};
+ data.forEach(row => {
+ const axisValue = row[axisLabel];
+ const metricValue = row[metricLabel];
+ if (typeof metricValue === 'number' && axisValue != null) {
+ const key = String(axisValue);
+ metricSums[key] = (metricSums[key] || 0) + metricValue;
+ }
+ });
Review Comment:
### Incorrect Metric Aggregation for Sorting <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The metric-based sorting implementation assumes summing is the appropriate
aggregation method for all metrics, which might not be correct for metrics like
averages, minimums, or maximums.
###### Why this matters
Using sum as the only aggregation method could lead to incorrect axis
ordering when the metric represents an average, minimum, maximum, or other
non-summable values.
###### Suggested change ∙ *Feature Preview*
Modify the code to respect the metric's aggregation type by adding an
aggregation parameter and implementing appropriate aggregation methods:
```typescript
const aggregateMetricValues = (values: number[], aggregationType: string) =>
{
switch (aggregationType) {
case 'avg':
return values.reduce((sum, val) => sum + val, 0) / values.length;
case 'min':
return Math.min(...values);
case 'max':
return Math.max(...values);
case 'sum':
default:
return values.reduce((sum, val) => sum + val, 0);
}
};
if (isMetricSort) {
const metricValues: Record<string, number[]> = {};
data.forEach(row => {
const axisValue = row[axisLabel];
const metricValue = row[metricLabel];
if (typeof metricValue === 'number' && axisValue != null) {
const key = String(axisValue);
metricValues[key] = metricValues[key] || [];
metricValues[key].push(metricValue);
}
});
const metricSums: Record<string, number> = {};
Object.entries(metricValues).forEach(([key, values]) => {
metricSums[key] = aggregateMetricValues(values, metric.aggregationType);
});
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c95cc65f-d615-487f-b49b-bc25520bd724/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c95cc65f-d615-487f-b49b-bc25520bd724?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c95cc65f-d615-487f-b49b-bc25520bd724?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c95cc65f-d615-487f-b49b-bc25520bd724?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c95cc65f-d615-487f-b49b-bc25520bd724)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:879daad4-de0c-473c-8702-a3ab849d3b86 -->
[](879daad4-de0c-473c-8702-a3ab849d3b86)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
