rebenitez1802 commented on code in PR #45311:
URL: https://github.com/apache/superset/pull/45311#discussion_r4326075405
##########
superset/charts/client_processing.py:
##########
@@ -752,13 +777,13 @@ def collapse(block: pd.DataFrame) -> tuple[pd.DataFrame,
str]:
else None,
)
- # if we want to apply the metrics on the rows we need to pivot the
- # dataframe back
+ # Replace placeholders before transposing, while column dtypes are intact.
+ df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
if apply_metrics_on_rows:
- df = df.T
+ # Mixed blank columns must not coerce integer cells through float64
+ # during transposition (which would round integers above 2**53).
+ df = df.astype(object).T if mixed_metric_totals else df.T
Review Comment:
🟡 Medium — Metrics-on-rows CSV ignores `CSV_EXPORT` `decimal` /
`float_format` (non-blocking, quick follow-up)
`DataFrame.to_csv` applies `decimal` and `float_format` only to float
columns. After this `astype(object)`, every output column is object, so every
float cell in the export, not only the totals, is written with `.`. That undoes
#38616 for this layout. It triggers with `metricsLayout: ROWS`, 2+ metrics,
column totals on, and float values (AVG metrics, Average/Median aggregation, or
any "Show values as" percent mode), for both CSV downloads and scheduled CSV
reports. The COLUMNS layout, single-metric pivots, Excel and JSON are
unaffected, as are deployments on the default `CSV_EXPORT`.
Repro with `CSV_EXPORT = {"sep": ";", "decimal": ","}`, two AVG metrics,
both totals on:
- master: `AVG(a);UK;3,5;3,5`
- this branch: `AVG(a);UK;3.5;3.5`
With `float_format="%.2f"`, master writes `3.50` and this branch writes
`3.5`.
Casting only when an integer column actually needs exact values fixes the
float case. I ran this change locally: `tests/unit_tests/charts/` plus
`excel_tests.py` give 479 passed, 2 xfailed, including the new large-integer
CSV/XLSX tests and a `CSV_EXPORT` decimal repro.
```suggestion
exact_ints = bool(mixed_metric_totals) and any(
pd.api.types.is_integer_dtype(dtype) for dtype in df.dtypes
)
df = df.astype(object).T if exact_ints else df.T
```
Worth adding a `CSV_EXPORT` sep/decimal test for ROWS layout with column
totals. A ROWS pivot that mixes integer and float metrics would still lose
`decimal`; converting columns to nullable `Int64`/`Float64` after the transpose
would close that too, but a blanket `convert_dtypes` breaks existing tests, so
that can be a separate follow-up.
--
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]
