Repository navigation
Conversation
Chart-data CSV and Excel exports serialize raw numeric values and ignore the number, currency and small-number formats configured on the chart, so exported cells do not match what the chart displays (e.g. 19.7999992 instead of 19.8). Fixes apache#45660. Add a reusable apply_export_number_formats helper that resolves each numeric column's configured d3 format, currency and small-number format the same way the Table plugin does -- column_config overrides first, then the datasource's saved per-metric formats -- and applies it through the existing format_column helper. Columns with nothing configured fall back to fixed-point text instead of raw floats; non-numeric columns, nulls, NaN and infinity pass through unchanged. The behavior is gated behind a new EXPORT_FORMATTED_NUMBERS feature flag (default False), so exports are unchanged unless it is explicitly enabled. Signed-off-by: CloudGeometry <eugeneo@cloudgeometry.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| # treats as unset. | ||
| small_number_format = (config.get("d3SmallNumberFormat") or "").strip() or None | ||
|
|
||
| if number_format or currency.get("symbol") or small_number_format: |
There was a problem hiding this comment.
Suggestion: Percent metrics default to PERCENT_3_POINT in the Table chart, but this condition sends them to the unconfigured fallback and exports fractions such as 0.3.
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Logic error
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/charts/client_processing.py
**Line:** 1290:1290
**Comment:**
*Logic Error: Percent metrics default to `PERCENT_3_POINT` in the Table chart, but this condition sends them to the unconfigured fallback and exports fractions such as `0.3`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of e4c3bce.
Percent metric columns are detected by their prefixed labels and passed to format_column with PERCENT_3_POINT as the default format.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 8faccdf.
Percent metric columns are identified by their prefixed labels and formatted with number_format or PERCENT_3_POINT instead of falling through to the fixed-point fallback.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of dc15753.
Percent metric columns are identified by their prefixed labels and formatted with number_format or PERCENT_3_POINT instead of falling through to the fixed-point fallback.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of a934729.
Percent metric columns are now detected by their prefixed labels and formatted with PERCENT_3_POINT when no number format is configured.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 78351a9.
Percent metric columns are identified by their prefixed labels and explicitly formatted with PERCENT_3_POINT when no number format is configured.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of ecb3342.
Percent metric columns are identified by their prefixed labels and explicitly formatted with PERCENT_3_POINT when no number format is configured.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 5180710.
Percent metric columns are identified by their prefixed labels and explicitly formatted with PERCENT_3_POINT when no number format is configured.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
Handled in the current revision. apply_export_number_formats matches percent-metric columns by their %-prefixed label (percent_metric_labels) and passes number_format or PERCENT_3_POINT to format_column, so a percent metric defaults to PERCENT_3_POINT like the Table plugin instead of falling through to the fixed-point fallback and exporting a fraction.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of e944e0a.
Percent-metric columns are identified by their prefixed labels and formatted with number_format or PERCENT_3_POINT instead of using the unconfigured fallback.
If that's not right, unresolve this thread and CodeAnt will leave it open.
| format_column( | ||
| df, | ||
| column, | ||
| number_format, | ||
| currency, | ||
| small_d3_format=small_number_format, | ||
| small_currency=small_currency, | ||
| ) |
There was a problem hiding this comment.
Suggestion: AUTO is passed without detected currency or row currency context, so it remains unresolved and exports amounts without the configured currency symbol.
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Api mismatch
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/charts/client_processing.py
**Line:** 1291:1298
**Comment:**
*Api Mismatch: `AUTO` is passed without detected currency or row currency context, so it remains unresolved and exports amounts without the configured currency symbol.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
There was a problem hiding this comment.
Agreed—this format_column call gets neither detected_currency nor a row currency context, whereas table() passes both, so an AUTO currency (from column_config or a saved metric) resolves to no symbol: 1234.5 with ,.2f + AUTO over a USD currency column exports 1,234.50 while the chart shows the $ symbol. Should the export resolve AUTO the way table() does?
There was a problem hiding this comment.
Yes. This is a real chart/export mismatch: these calls pass the currency configuration but omit the detected currency and row-currency context that table() uses, so AUTO cannot resolve to the chart’s symbol.
The export helper should receive and forward the same context as table() when calling format_column. Add a regression test with an AUTO format and USD currency context, asserting the export includes $ just as the chart does. If that context is unavailable for an export, it should follow table()’s fallback behavior rather than silently claiming to match the chart.
There was a problem hiding this comment.
Done in 8faccdf. apply_export_number_formats now mirrors table(): it builds a per-row currency context from the datasource's currency-code column and passes it to format_column, with the query-wide single currency (detected from the frame already in hand, no extra query) as the fallback_to_detected=False fallback. An AUTO currency now resolves to the row's symbol ($1,234.50) instead of exporting bare. Added test_auto_currency_resolves_from_row_context.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 8faccdf.
The formatting call now receives both detected_currency and row_currency_context, allowing AUTO currency formats to use the detected currency or per-row currency.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 78351a9.
format_column now receives both detected_currency and row_currency_context for configured currency formats.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of ecb3342.
format_column now receives both detected_currency and row_currency_context for configured currency formats.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 5180710.
format_column now receives both detected_currency and row_currency_context for configured currency formats.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
Handled in the current revision. apply_export_number_formats resolves AUTO the way table() does: a per-row currency context from the datasource currency-code column (row_currency_context), a query-wide detected_currency gated on auto_currency_configured, and the top-level currency_format fallback, all forwarded to format_column. So an AUTO currency exports with its symbol rather than a bare amount.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of e944e0a.
The formatting call now receives both detected_currency and row_currency_context when formatting configured currency columns.
If that's not right, unresolve this thread and CodeAnt will leave it open.
| apply_export_number_formats( | ||
| df, | ||
| coltypes, | ||
| self._query_context.form_data or {}, | ||
| self._qc_datasource, | ||
| ) |
There was a problem hiding this comment.
Suggestion: On XLSX exports, excel.apply_column_types converts formatted strings like 3.50 back to numeric 3.5, so the exported cell loses its configured decimal places.
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Type error
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/common/query_context_processor.py
**Line:** 650:655
**Comment:**
*Type Error: On XLSX exports, `excel.apply_column_types` converts formatted strings like `3.50` back to numeric `3.5`, so the exported cell loses its configured decimal places.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of e4c3bce.
For formatted XLSX exports, numeric columns are passed to excel.apply_column_types as strings, preventing formatted values such as 3.50 from being coerced back to numbers.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 8faccdf.
For formatted numeric exports, numeric column types are changed to GenericDataType.STRING before excel.apply_column_types, preserving the formatted decimal places in XLSX.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of dc15753.
For formatted numeric exports, numeric column types are changed to GenericDataType.STRING before excel.apply_column_types, preserving the formatted decimal places in XLSX.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of a934729.
For XLSX exports with formatted numeric values, numeric column types are changed to strings before apply_column_types, preserving configured decimal places.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 78351a9.
For formatted XLSX exports, numeric column types are changed to string before apply_column_types, preserving the formatted decimal places.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of ecb3342.
For formatted XLSX exports, numeric column types are changed to string before apply_column_types, preserving the formatted decimal places.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 5180710.
For formatted XLSX exports, numeric column types are changed to string before apply_column_types, preserving the formatted decimal places.
If that's not right, unresolve this thread and CodeAnt will leave it open.
There was a problem hiding this comment.
Handled in the current revision. In the XLSX branch of get_data, with the flag on, every numeric column is remapped to GenericDataType.STRING before apply_column_types precisely so "3.50" is not coerced back to the number 3.5. Covered by test_xlsx_export_keeps_configured_decimals_as_text, which reads the raw cell and asserts data_type 's' with value '3.50'.
There was a problem hiding this comment.
✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of e944e0a.
For formatted XLSX exports, numeric columns are passed to apply_column_types as strings, preserving rendered decimal places.
If that's not right, unresolve this thread and CodeAnt will leave it open.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #45817 +/- ##
===========================================
- Coverage 85.03% 65.41% -19.63%
===========================================
Files 3080 3080
Lines 209437 209589 +152
Branches 49385 49402 +17
===========================================
- Hits 178095 137097 -40998
- Misses 27960 69360 +41400
+ Partials 3382 3132 -250
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Apply ruff format to the two new unit test files and regenerate docs/static/feature-flags.json so it includes the new EXPORT_FORMATTED_NUMBERS flag, fixing the pre-commit CI job. Signed-off-by: CloudGeometry <eugeneo@cloudgeometry.com>
…rs' into fix/45660-export-formatted-numbers
get_data() is typed as str | bytes | list[dict], so the export tests needed to narrow that union to bytes before decoding / reading XLSX, and the generic dict/list annotations needed type parameters. Also cast the Explorable datasource to the helper's BaseDatasource | Query | None at the single call site in get_data(). Signed-off-by: CloudGeometry <eugeneo@cloudgeometry.com>
…rs' into fix/45660-export-formatted-numbers
Resolve the correctness and scope issues raised in review: - Do not double-process POST_PROCESSED exports. get_data() now skips the pre-serialization formatting for the post-processed path, where apply_client_processing re-parses the CSV and runs pivot/table arithmetic; formatting a thousands-separated value there coerced the pivot totals to NaN. The tabular export formats are instead formatted after post-processing, reusing the existing table()/pivot_table_v2() formatters. - Keep XLSX cells as text once formatted, so apply_column_types no longer coerces e.g. "3.50" back to the number 3.5 and drops the decimals. - Format finite floats and large integers as exact fixed-point text in the unconfigured fallback, so 1e-07 / 19.7999992 no longer leak and large ints are not rounded by a float64 re-inference. - Honor the AG Grid small-number exemption and default percent-metric columns to PERCENT_3_POINT, mirroring the live Table plugin. - Document the flag's scope: it does not cover the streaming CSV export path, and it only resolves the per-column formats a table-like viz carries (not chart-level keys such as a Time-series y_axis_format). Add tests pinning config-over-saved-format precedence, small-number forwarding, the float / large-int fallback, the AG Grid and percent-metric behavior, the XLSX decimal retention, and the post-processed skip. Signed-off-by: CloudGeometry <eugeneo@cloudgeometry.com>
- Only skip pre-serialization formatting for POST_PROCESSED exports whose viz_type has a post-processor (table / pivot_table_v2); other viz types (e.g. ag-grid-table in scheduled reports/alerts) have no post-processor, so apply_client_processing returns them unchanged and they must still be formatted in get_data. - Resolve an AUTO currency in apply_export_number_formats the way table() does: a per-row currency context from the datasource currency-code column with the query-wide single currency (detected from the frame in hand) as fallback, so an AUTO currency exports with its symbol instead of bare. - Tests: ag-grid POST_PROCESSED is formatted in get_data; round-trip proving apply_client_processing formats the aggregate after get_data skips it; AUTO currency resolves its symbol from the row context. Signed-off-by: CloudGeometry <eugeneo@cloudgeometry.com>
The non-numeric/null cell test pinned the string column's null to None, but under the pandas future.infer_string option a string column infers dtype str and its null becomes NaN, so the DataFrame constructor itself (not this helper) decides the marker. Assert the non-null cells are kept and the null stays null via pd.isna, independent of the None/NaN representation. Signed-off-by: CloudGeometry <eugeneo@cloudgeometry.com>
| if coltype == GenericDataType.NUMERIC | ||
| } | ||
|
|
||
| for column in df.columns: |
There was a problem hiding this comment.
If two numeric metrics share a verbose name (say both display as Total), get_data() renames both columns before this helper runs, so df[column] here is a two-column DataFrame. format_column then either hands whole Series to the per-cell formatter (the configured ,.2f is silently skipped and raw values ship) or, when a currency-code column is selected, zips the two columns against the per-row contexts and the strict=True zip raises ValueError, failing the whole CSV/XLSX export. Could the loop go by column position (or skip and dedupe duplicate labels) so a repeated verbose name can't change or abort the export?
There was a problem hiding this comment.
Fixed in a934729. format_column now addresses the column by position (df.iloc[:, col_pos] / df.isetitem) instead of by label, so when two metrics share a verbose name the repeated label no longer turns df[label] into a two-column frame — it no longer silently skips the configured format, nor raises on the strict=True zip and aborts the export. Added two regression tests (the plain and the per-row-currency paths) that fail on the old label-based access.
There was a problem hiding this comment.
Positional writes fix the column lookup, but two same-label metrics still lose their own formats and values. number_format_by_label, currency_by_label and config_by_label are keyed by label, so with a saved as .2f and b as .0%, both shown as Total, a=1.5 exports as 150% instead of 1.50. For CSV the formatted columns are now strings, and df_to_escaped_csv writes them back with df.at[label, name], which assigns the first value to both same-named columns (1.50,2.50 exports as 1.50,1.50; I reproduced it on a two-column frame). Should format resolution key off the original column identity, and the string write-back avoid label-based .at?
There was a problem hiding this comment.
Done. apply_export_number_formats now iterates for col_pos, column in enumerate(df.columns) and addresses each column positionally (format_column(df, col_pos, ...), df.isetitem(col_pos, ...)), and format_column takes a positional col_pos: int. So two metrics sharing a verbose name no longer collapse into a two-column df[label] that skips formatting or aborts the export on the strict=True zip.
- Format export columns by position, not by label. Two metrics can share a verbose name, so get_data() renames distinct columns to the same label; a label lookup then returned a two-column frame and either silently skipped the configured format or raised on zip(strict=True), aborting the whole CSV/XLSX export. format_column now takes the column position and reads/writes via df.iloc / df.isetitem, so a repeated verbose name formats each column once. Tests: - per-row AUTO currency with mixed USD/EUR rows, asserting each row's own symbol (fails if the per-row currency context is dropped); - two regression tests for duplicate verbose names (plain and currency paths); - XLSX export reads the raw cell via openpyxl and asserts the ",.2f" value is the text "3.50" (data_type "s"), pinning that apply_column_types does not coerce the formatted string back to the number 3.5. Signed-off-by: Eugene Olkhovski <eugeneo@cloudgeometry.com>
| def format_column( | ||
| df: pd.DataFrame, | ||
| column: Any, | ||
| col_pos: int, |
There was a problem hiding this comment.
Switching format_column to a positional col_pos leaves its existing callers in tests/unit_tests/charts/test_client_processing.py (around lines 3915-3942) passing "amount", so df.iloc[:, "amount"] raises a ValueError. That is why unit-tests (current) fails on three test_format_column_* tests, with the flag off. Should those callers pass the column position (0)?
There was a problem hiding this comment.
Fixed. The direct format_column callers in test_client_processing.py (~lines 3945-3972) now pass the column position 0 instead of "amount", so the three test_format_column_* tests pass with the flag off. Verified locally: pytest tests/unit_tests/charts/test_client_processing.py -k format_column -> 4 passed.
| text = _csv_text(result) | ||
| # Sub-one value rendered by the small-number format, the other by the main. | ||
| assert "12.5%" in text | ||
| assert "5" in text |
There was a problem hiding this comment.
assert "5" in text is already satisfied by the 12.5% on the line above, so it does not show the 5.0 row takes the main ,.0f format; if that cell stayed 5.0 or disappeared the test would still pass. Could you parse the CSV and assert the amount cells are exactly ["12.5%", "5"]?
There was a problem hiding this comment.
Good point. test_small_number_format_forwarded_to_export now parses the CSV and asserts the amount column is exactly ['12.5%', '5'] (pd.read_csv(io.StringIO(text), dtype=str)), so a 5.0 or a dropped cell would fail. In e944e0a.
The dup-verbose-name fix changed format_column's second parameter from a column label to a positional index (col_pos: int). The pre-existing direct-call unit tests still passed the label 'amount', so df.iloc[:, 'amount'] raised ValueError and failed unit-tests CI. Address the positional signature (single-column frames -> position 0); the df['amount'] read-back assertions are unchanged. Signed-off-by: Eugene Olkhovski <eugeneo@cloudgeometry.com>
| # treat them as strings here; otherwise apply_column_types | ||
| # would coerce e.g. "3.50" back to the number 3.5 and drop | ||
| # the configured decimals. | ||
| export_coltypes = [ |
There was a problem hiding this comment.
Mapping every numeric column to STRING here sends negative numbers through excel._quote_formula, because - is in FORMULA_PREFIXES. With the flag on, an unconfigured -3.5 (or -3.50 under ,.2f) is stored as the literal text '-3.5, so the apostrophe shows up as cell content and the value is no longer numeric. Should the formula quoting be skipped for the formatted numeric columns, or should only configured columns become text?
There was a problem hiding this comment.
Fixed in e944e0a. The number-formatted column positions are threaded into df_to_excel -> quote_formulas, which exempts a formatted negative (-3.50) in those columns from formula-quoting, matching the CSV writer's escape_value/_is_negative_number. So it is stored as -3.50 (data_type 's'), not the literal '-3.50. A genuine formula prefix (=/+/@) in those columns, and every other column (headers, warehouse strings), still quote exactly as before: the exemption is an opt-in per-column flag, so the base _quote_formula/df_to_excel behavior and the injection guard are unchanged. New tests: XLSX and CSV keep -3.50 unquoted, and a warehouse =1+1 string is still quoted.
| "Total (Sum)": "15", | ||
| } | ||
| assert unformatted["big"].tolist() == df["big"].tolist() | ||
| # Licensed to the Apache Software Foundation (ASF) under one |
There was a problem hiding this comment.
This file shows as rewritten in full (5794 lines changed) because the last commit saved it with CRLF line endings; on master it is LF. That buries the five real format_column(df, 0, ...) edits near line 3915 and will make blame and rebases noisy. Could you restore LF endings so the diff is only the call-site changes?
There was a problem hiding this comment.
Fixed. The test file is back to LF (the committed blob has no CRLF), so the diff is just the five format_column(df, 0, ...) call-site edits rather than a full-file rewrite. Re-pushed clean.
github-actions
Bot
added
the
requires:rebase
Resolve conflicts from apache#45865 (pandas 3 fixes) in superset/charts/client_processing.py and its unit test: - format_column: keep the positional col_pos addressing (this PR's dup-verbose-name fix) over master's label-based df[column] apply; take master's pivot_df SUPERSET_PANDAS_NAN replace fix as-is. - test_client_processing.py: re-base on master's copy (restoring LF line endings after an earlier accidental CRLF rewrite) and re-apply only the five positional format_column call updates. Signed-off-by: Eugene Olkhovski <eugeneo@cloudgeometry.com>
|
Synced the branch with Resolution:
The branch is mergeable again; re-running CI after the sync. |
…xport Addresses the open apache#45817 review thread on the XLSX export path. With EXPORT_FORMATTED_NUMBERS on, numeric columns are rendered to display text and typed as strings so apply_column_types keeps their decimals. That routed a formatted negative like -3.50 through excel._quote_formula, which (because '-' is in FORMULA_PREFIXES) stored it as the literal text '-3.50 and dropped its numeric meaning. The CSV writer already exempts negative numbers (escape_value / _is_negative_number); XLSX did not, so the two diverged. Thread the number-formatted column positions into df_to_excel / quote_formulas and exempt those columns' formatted negatives from quoting, matching CSV. A genuine formula prefix (=/+/@) in those columns, and every other column (headers, warehouse strings), still quote exactly as before, so the base df_to_excel behavior and formula-injection guard are unchanged. Tests: XLSX and CSV keep a formatted -3.50 unquoted; a warehouse '=1+1' string is still quoted; and the small-number-format test now parses the column and pins both cells exactly instead of a substring match. Signed-off-by: Eugene Olkhovski <eugeneo@cloudgeometry.com>
|
@sadpandajoe thanks for the detailed review. All open threads are addressed; the branch is synced with master and green. New in e944e0a:
Already in the current revision (replied inline on each):
Ran locally: |
github-actions
Bot
removed
the
requires:rebase
SUMMARY
Chart-data CSV and Excel exports serialize raw numeric values and ignore the
number, currency and small-number formats configured on the chart, so exported
cells do not match what the chart displays (for example
19.7999992instead of19.8). Fixes #45660.This adds a reusable
apply_export_number_formatshelper insuperset/charts/client_processing.pythat resolves each numeric column'sconfigured d3 format, currency and small-number format the same way the Table
viz does --
column_configoverrides first, then the datasource's savedper-metric formats -- and applies it through the existing
format_columnhelper. Columns with nothing configured fall back to fixed-point text instead
of raw floats; non-numeric columns, nulls, NaN and infinity pass through
unchanged. The SQL Lab export path is left untouched.
The behavior is gated behind a new
EXPORT_FORMATTED_NUMBERSfeature flag(default
False), so existing exports are byte-for-byte unchanged unless it isexplicitly enabled.
TESTING INSTRUCTIONS
EXPORT_FORMATTED_NUMBERStoTruein your feature flags.currency on it.
flag off, the exported bytes are unchanged.
New unit tests:
tests/unit_tests/charts/test_export_number_format.pytests/unit_tests/common/test_query_context_processor_export_format.pyADDITIONAL INFORMATION
EXPORT_FORMATTED_NUMBERS