korbit-ai[bot] commented on code in PR #32253:
URL: https://github.com/apache/superset/pull/32253#discussion_r1955034951
##########
superset/utils/pandas_postprocessing/histogram.py:
##########
@@ -60,8 +60,7 @@ def histogram(
# convert the bin edges to strings
bin_edges_str = [
- f"{int(bin_edges[i])} - {int(bin_edges[i+1])}"
- for i in range(len(bin_edges) - 1)
+ f"{bin_edges[i]} - {bin_edges[i+1]}" for i in range(len(bin_edges) - 1)
Review Comment:
### Uncontrolled decimal precision in bin labels <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The bin edge values are now displayed with full decimal precision, which can
lead to excessively long and unwieldy labels with many decimal places.
###### Why this matters
Without proper decimal place formatting, bin labels could become very long
(e.g., '3.141592653589793 - 4.141592653589793'), making the visualization
harder to read and interpret.
###### Suggested change ∙ *Feature Preview*
Add decimal place formatting to the bin edge values:
```python
f"{bin_edges[i]:.2f} - {bin_edges[i+1]:.2f}"
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0efb854b-626f-474b-a2f3-cef3993db06a?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:ab44e544-b543-4097-8635-86ce1147a214 -->
##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts:
##########
@@ -58,19 +59,33 @@ export default function transformProps(
showLegend,
showValue,
sliceId,
+ xAxisFormat,
xAxisTitle,
yAxisTitle,
+ yAxisFormat,
} = formData;
const { data } = queriesData[0];
const colorFn = CategoricalColorNamespace.getScale(colorScheme);
- const formatter = getNumberFormatter(
- normalize ? NumberFormats.FLOAT_2_POINT : NumberFormats.INTEGER,
- );
+
+ const formatter = (format: string) =>
+ getValueFormatter(
+ column,
+ currencyFormats,
+ columnFormats,
+ format,
+ undefined,
+ );
+ const xAxisFormatter = formatter(xAxisFormat);
+ const yAxisFormatter = formatter(yAxisFormat);
+
const percentFormatter = getPercentFormatter(NumberFormats.PERCENT_2_POINT);
const groupbySet = new Set(groupby);
- const xAxisData: string[] = Object.keys(data[0]).filter(
- key => !groupbySet.has(key),
- );
+ const xAxisData: string[] = Object.keys(data[0])
+ .filter(key => !groupbySet.has(key))
+ .map(key => {
+ const array = key.split(' - ').map(value => parseFloat(value));
Review Comment:
### Missing Error Handling for Bin Edge Parsing <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code assumes bin edges will always be valid numbers that can be parsed
with parseFloat, without any error handling for malformed data.
###### Why this matters
If the data contains malformed bin edges that can't be parsed to floats, it
will result in NaN values and potentially crash the visualization or display
incorrect data.
###### Suggested change ∙ *Feature Preview*
Add error handling to safely parse bin edges and provide fallback values if
needed:
```typescript
const array = key.split(' - ').map(value => {
const parsed = parseFloat(value);
if (Number.isNaN(parsed)) {
console.warn(`Invalid bin edge value: ${value}`);
return 0; // or other appropriate fallback
}
return parsed;
});
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8cdebb56-dcdd-4b25-89fe-6acf6eca5c11?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:a29b20b6-84cf-4616-bd26-7775b8242ce3 -->
--
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]
