SEPURI-SAI-KRISHNA commented on code in PR #45439:
URL: https://github.com/apache/superset/pull/45439#discussion_r4335032246
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -2501,6 +2501,318 @@ 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.
+
+ The switch point itself is covered on both sides and in both signs. The
+ browser's test is ``Math.abs(value) < 1``, so exactly one is a large cell
+ and a large negative stays large: widening the comparison to ``<= 1`` or
+ comparing the signed value would render one of these as a percentage.
+ """
+ df = pd.DataFrame.from_dict(
+ {
+ "ratio": {
+ 0: 0.0523,
+ 1: 12.5,
+ 2: -0.5,
+ 3: 0.0,
+ 4: 1.0,
+ 5: -1.0,
+ 6: -12.5,
+ }
+ }
+ )
+ 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%",
+ "1.00",
+ "-1.00",
+ "-12.50",
+ ]
+
+
+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"]
Review Comment:
You are right, and `1200.0` is the value. Measured both ways:
| second cell | metric | numeric dimension |
| --- | --- | --- |
| `12.5` | `12.5` | `12.5` |
| `1200.0` | `1.2k` | `1200` |
`format_number_with_config(None, {}, 1200.0)` is `'1200'` and SMART_NUMBER is
`'1.2k'`, so the dimension half now reads `["5.2%", "1200"]` and dropping
`column in metric_labels` fails it.
Running the same check over the rest of the diff turned up two more
conditions
with nothing behind them, both in the same commit.
The type check in `is_small_number` is load-bearing, and not only for
matching
the browser's `typeof`. The percent metric branch is the one caller that does
not test `is_numeric_dtype` first, so a percent metric column holding a null
or
a text value arrives at the predicate, and `float` raises `TypeError` on the
first and `ValueError` on the second. That is a failed report rather than a
misformatted cell.
`test_table_small_number_format_on_a_non_numeric_percent_metric` covers null,
text and NaN, built as an object column on purpose since `from_dict` would
turn
the null into a NaN in a float column.
The `or SMART_NUMBER` after the `$` strip is load-bearing too. A small-number
format that is nothing but `$` strips to the empty string, and
`CurrencyFormatter.format` calls
`getNumberFormatter(getNormalizedD3Format())`,
whose registry answers an empty id with its `defaultKey`, SMART_NUMBER
(`NumberFormatterRegistry.get`, `formatterId === ''`). Without the fallback
the
small-number branch is skipped and the cell takes the column's normal format:
`0.50` where the chart shows `0.5`.
`test_table_small_number_format_of_only_a_dollar_falls_back` covers it.
Two conditions in `is_small_number` turned out to be dead and are gone, along
with the `math` import: `abs` already puts NaN and infinity outside
`abs(value) < 1`, exactly as `Math.abs` does, so the explicit `isnan` and
`isinf` checks could not change an answer. The body is now
`return abs(float(value)) < 1`.
Two are left with no test because no input separates them, and they are in
the
description as kept for what they say rather than what they do: excluding
`bool` (the browser's `typeof value === 'number'`, unobservable here only
because `format_number_with_config` passes booleans through untouched either
way) and mapping a blank format to `None` rather than `""` (both falsy at
every
use, so `None` is only the typed choice).
--
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]
