Copilot commented on code in PR #38345:
URL: https://github.com/apache/superset/pull/38345#discussion_r2885552533


##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/types.ts:
##########
@@ -31,6 +31,7 @@ export type HistogramFormData = QueryFormData & {
   xAxisFormat: string;
   xAxisTitle: string;
   yAxisFormat: string;
+  yAxisLogScale: boolean;

Review Comment:
   `yAxisLogScale` is typed as a required boolean, but existing saved slices 
(and programmatic consumers) may not have this key in `formData` yet. Making 
this field optional (or providing a default in `transformProps` via 
destructuring) avoids forcing an incompatible type assumption and prevents 
future refactors from treating it as always-present.
   ```suggestion
     yAxisLogScale?: boolean;
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts:
##########
@@ -169,9 +170,10 @@ export default function transformProps(
     },
     yAxis: {
       ...defaultYAxis,
+      ...(yAxisLogScale ? { min: 1 } : {}),

Review Comment:
   On a log Y-axis, values must be > 0. Setting `min: 1` can also break 
`normalize` mode (normalized frequencies are typically < 1) and will clip or 
misrender bins with 0 counts. Consider deriving a positive `min` from the 
smallest non-zero datum (or using `min: 'dataMin'` with explicit handling for 
zeros), and ensure series values <= 0 are mapped to `null`/filtered (or 
otherwise handled) when `yAxisLogScale` is enabled.
   ```suggestion
         ...(yAxisLogScale
           ? {
               // On a log axis, ensure a strictly positive minimum derived 
from data.
               // Use ECharts' computed data min when it's > 0, otherwise fall 
back to a tiny positive value.
               min: (value: { min: number }) =>
                 value.min > 0 ? value.min : Number.MIN_VALUE,
             }
           : {}),
   ```



-- 
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]

Reply via email to