sadpandajoe commented on code in PR #45439:
URL: https://github.com/apache/superset/pull/45439#discussion_r4317793072
##########
superset/charts/client_processing.py:
##########
@@ -1246,15 +1302,37 @@ def label_of(name: str) -> str:
if configured_currency.get("symbol")
else currency_by_label.get(column) or {}
)
+ # The small-number format has no dataset-level counterpart, and the
+ # browser pairs it with the column config's own currency rather than
+ # the resolved one, so both come straight from ``config``. A cleared
+ # Select leaves a blank string behind, which the browser treats as
+ # unset.
+ small_number_format = (config.get("d3SmallNumberFormat") or
"").strip() or None
is_number = pd.api.types.is_numeric_dtype(df[column])
if column in percent_metric_labels:
- format_column(df, column, number_format or PERCENT_3_POINT, {})
+ format_column(
+ df,
+ column,
+ number_format or PERCENT_3_POINT,
+ {},
+ small_d3_format=small_number_format,
+ small_currency=configured_currency,
Review Comment:
This percent-metric call passes `small_currency` but no `currency_context`,
so `format_column` takes its no-context branch and hands the raw `AUTO` config
to the small-number formatter. For a table with a currency column, a `%cnt`
cell of 0.5 on a USD row, and `%cnt` configured with `d3SmallNumberFormat:
",.3f"` plus an AUTO prefix currency, the other numeric-column branch renders
`$ 0.500` (per-row context) but this one renders `0.500`, even though the
`format_column` docstring says AUTO resolves against the row's context on both
paths. Should this branch pass `row_currency_context` and
`fallback_to_detected=False` like the numeric-column call below?
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -2501,6 +2501,118 @@ def test_table_applies_si_number_format() -> None:
assert formatted["amount"].tolist() == ["1.23k"]
+def test_table_applies_small_number_format() -> None:
+ """
+ A cell below one in absolute value takes the column's small-number format.
+
+ ``formatColumnValue`` in the Table plugin picks the formatter per value, so
+ the report has to switch per cell rather than per column.
+ """
+ df = pd.DataFrame.from_dict({"ratio": {0: 0.0523, 1: 12.5, 2: -0.5, 3:
0.0}})
+ form_data = {
+ "viz_type": "table",
+ "metrics": ["ratio"],
+ "column_config": {
+ "ratio": {"d3NumberFormat": ",.2f", "d3SmallNumberFormat": ".1%"}
+ },
+ }
+ formatted = table(df, form_data)
+ assert formatted["ratio"].tolist() == ["5.2%", "12.50", "-50.0%", "0.0%"]
+
+
+def test_table_ignores_blank_small_number_format() -> None:
+ """
+ A cleared Select leaves an empty string behind, which
+ ``getSmallNumberFormatter`` treats as unset.
+ """
+ df = pd.DataFrame.from_dict({"ratio": {0: 0.0523, 1: 12.5}})
+ form_data = {
+ "viz_type": "table",
+ "metrics": ["ratio"],
+ "column_config": {
+ "ratio": {"d3NumberFormat": ",.2f", "d3SmallNumberFormat": " "}
+ },
+ }
+ formatted = table(df, form_data)
+ assert formatted["ratio"].tolist() == ["0.05", "12.50"]
+
+
+def test_table_small_number_format_without_a_normal_format() -> None:
+ """
+ A small-number format on its own is enough to format the small cells.
+
+ The other cells follow the browser: a metric falls back to SMART_NUMBER,
+ while a numeric dimension gets no formatter at all and is printed as is.
+ """
+ df = pd.DataFrame.from_dict({"ratio": {0: 0.0523, 1: 1200.0}})
+ form_data = {
+ "viz_type": "table",
+ "metrics": ["ratio"],
+ "column_config": {"ratio": {"d3SmallNumberFormat": ".1%"}},
+ }
+ assert table(df, form_data)["ratio"].tolist() == ["5.2%", "1.2k"]
+
+ df = pd.DataFrame.from_dict({"ratio": {0: 0.0523, 1: 12.5}})
+ form_data = {
+ "viz_type": "table",
+ "metrics": [],
+ "column_config": {"ratio": {"d3SmallNumberFormat": ".1%"}},
+ }
+ assert table(df, form_data)["ratio"].tolist() == ["5.2%", "12.5"]
+
+
+def test_table_small_number_format_uses_the_column_config_currency() -> None:
+ """
+ ``getSmallNumberFormatter`` pairs the small format with the column config's
+ own currency, and the currency formatter drops percent signs.
+ """
+ df = pd.DataFrame.from_dict({"amount": {0: 0.5, 1: 12.5}})
+ form_data = {
+ "viz_type": "table",
+ "metrics": ["amount"],
+ "column_config": {
+ "amount": {
+ "d3NumberFormat": ",.2f",
+ "d3SmallNumberFormat": ".1%",
+ "currencyFormat": {"symbol": "USD", "symbolPosition":
"prefix"},
+ }
+ },
+ }
+ formatted = table(df, form_data)
Review Comment:
This test sets USD at the column level, so the normal and small formatters
receive identical currency configs. It would still pass if the small path
started using the resolved `currency` (which falls back to the dataset-saved
currency) instead of the column config's own `configured_currency`. Could we
add a case with a dataset-saved USD currency, no column-level `currencyFormat`,
formats `,.2f` / `.1%`, and values `[0.5, 12.5]`, asserting `["50.0%", "$
12.50"]`, so the column-config-only pairing is actually pinned?
--
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]
