sadpandajoe opened a new pull request, #42820:
URL: https://github.com/apache/superset/pull/42820
### SUMMARY
When an Alert/Report sends a Table or Pivot Table chart **as text**
(embedded in the email body), the number and currency formatting configured on
the chart was not applied, so the email showed raw values.
`superset/charts/client_processing.py` reproduces the chart's client-side
formatting on the server for reports. For Table it attempted only
`d3NumberFormat`, by pasting the d3 format string straight into Python's
`str.format` mini-language — a different grammar — wrapped in a bare `except:
pass` whose comment reads "if we can't format the column for any reason, send
as is".
The two grammars overlap only narrowly, so most specifiers raised and
silently fell back to the raw value: `.1s`, `$,.2f`, `~g` and the default
`SMART_NUMBER` all fail outright. Even `,d` works only for `int` — it raises
`Unknown format code 'd'` on `float`, and pandas metric columns are `float64`,
so it failed for ordinary counts and sums too. Specs valid in both grammars,
such as `,.2f`, were the only ones that came through. Nothing was logged, so
the failure was invisible rather than merely wrong.
`currencyFormat` was never read on that path at all, and `pivot_table_v2()`
applied no formatting whatsoever.
This adds `superset/utils/number_format.py`, a Python port of the frontend
formatters — d3-format, `createSmartNumberFormatter.ts` and
`CurrencyFormatter.ts` — reusing Babel (already a dependency) for currency
symbols, and wires it into `table()` and `pivot_table_v2()`.
This supersedes #41028, which was opened against a fork that is no longer
maintained. The original work is carried here with attribution, plus the review
feedback that PR had accumulated. Thanks to @massucattoj for the original
implementation.
### WHAT'S INCLUDED BEYOND THE ORIGINAL
The commits are grouped by intent and are best read in order.
**Review feedback from #41028**
- **Saved metric formats are no longer dropped.** Both frontend plugins read
per-metric formats from the datasource's `column_formats`, while the server
read only `columnFormats` from form data — and `table()` ignored the datasource
entirely. Datasource formats are now merged with chart-level overrides, chart
winning, matching `plugin-chart-table/src/transformProps.ts` and
`PivotTableChart.tsx`.
- **Currency symbol placement respects the configured locale** instead of a
hard-coded `en_US`, so reports agree with Explore on non-English deployments.
Reports run in a Celery worker with no request context, so resolution falls
back through `BABEL_DEFAULT_LOCALE` to a safe default.
- **Per-row/per-cell currency context is supported.** The query already
returns the currency column and the payload retains it; the association was
being lost in `pivot_df`. A parallel pivot now carries the set of contributing
currencies through cells, totals and subtotals. A cell backed by a single
currency renders that symbol, a cell mixing currencies stays neutral, and an
empty context follows the existing fallback.
**Columns relying on the implicit default**
The browser gives every numeric metric column a formatter even when nothing
is configured — `getNumberFormatter(undefined)` resolves to the registry
default, `SMART_NUMBER`. Report text previously formatted only columns with an
explicit format, so an aggregate metric with no configured format showed
`1.23M` in Explore and `1234567` in the email. That is the same symptom this PR
targets, in its most common form.
Table now applies the same defaults as the plugin: `PERCENT_3_POINT` for
percent metrics, `SMART_NUMBER` for numeric metrics with no explicit format.
Numeric **dimension** columns are deliberately left untouched, matching
`transformProps.ts` — there is a test asserting a numeric dimension stays raw
while its sibling metric formats.
This gap is Table-specific in practice. The Pivot Table's `valueFormat`
control carries a `SMART_NUMBER` default that is persisted into saved form
data, so pivots created through the UI were already covered; Table's is a
*per-column* formatter the browser synthesizes at render time, so nothing is
ever persisted for it. Pivot still gets a small defensive fallback for charts
that never persisted the key (API-created or older), which is a no-op wherever
`valueFormat` is present.
**A crash, not just a formatting miss**
Carrying per-row currency through a pivot left the parallel currency
structure with `NaN` in the empty cells of a sparse cross-product, which raised
`TypeError` outside any error handling and failed report generation entirely. A
missing or non-iterable currency context is now coerced to the empty-context
path, which resolves the same way it always did (single currency → that symbol,
mixed → neutral, empty → detected fallback).
**d3 parity fixes**
These came from diffing the port against regenerated `d3-format` output
rather than from reading the Python:
- whole-valued floats no longer gain a trailing `.0` under `,` and `+,` —
pandas metric columns are `float64`, so this affected the common case of counts
and sums
- `f`/`%`/`e`/`s`/`r` use d3-compatible rounding rather than Python's
half-to-even, so money presets like `,.2f` and `$,.2f` match the browser
- default/`,` formatting follows d3's exponent thresholds instead of forcing
fixed-point
- `~g` keeps small values in fixed notation instead of switching to
scientific
- `.0s` clamps to one significant digit rather than rendering `0k`
**Failing loudly instead of quietly**
The specifier parser previously accepted d3 grammar it did not implement,
which is the worst case: a valid user-entered format silently rendering a
confidently wrong number. Accounting parentheses and space-sign are now
implemented; fill, align, zero-pad and width are rejected explicitly. Rejected
formats preserve the raw value, which is visible and correctable.
### KNOWN LIMITATION
The specialized preset families — `DURATION`, `DURATION_SUB`,
`DURATION_COL`, `MEMORY_DECIMAL`, `MEMORY_BINARY`, `MEMORY_TRANSFER_RATE_*` and
the length formatters — are **not** ported. They are rejected explicitly and
fall back to the raw value, so a chart using one of them still shows an
unformatted number in report text.
This is deliberate: those are locale-aware formatter factories rather than
d3 format strings, and porting them faithfully is a separate piece of work.
They are called out here rather than left to be discovered, since this PR does
not make report formatting complete — it makes the d3 and currency formats work
and makes the rest fail visibly instead of silently.
### TESTING INSTRUCTIONS
1. Create a Table or Pivot Table chart and configure a number format (e.g.
`.1s`, `,.2f`) and/or a currency on a column.
2. Open the endpoint reports use to build the embedded table:
`/api/v1/chart/<chart_id>/data/?type=post_processed&format=json`
Numeric columns with a configured format now render formatted (`8k`, `$
1,234.50`) instead of raw.
3. Full path: configure an Alert/Report to send the chart as text and
confirm the email body table is formatted.
4. Unit tests:
```
pytest tests/unit_tests/utils/number_format_test.py
tests/unit_tests/charts/test_client_processing.py
```
163 tests. The parity suite asserts against values captured from real
`d3-format` output rather than from this implementation.
CSV/XLSX attachments keep raw numeric values and column types for downstream
analysis — formatting applies to the JSON path only. This is a behavior change
from before (the old code formatted regardless of result format) and is noted
in `UPDATING.md`.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
