SEPURI-SAI-KRISHNA commented on PR #45439: URL: https://github.com/apache/superset/pull/45439#issuecomment-6250613535
Good catch, this was a real defect. Pushed a fix. I had reasoned that the small-number formatter gets no row context, so handing it the column config's currency as-is was enough. That is wrong. `formatValue` passes `rowData` and `currencyColumn` through whenever the formatter it picked is a `CurrencyFormatter`, and `getSmallNumberFormatter` returns exactly that when `config.currencyFormat` is set. So in the browser an AUTO currency does resolve per row on the small-number path too, and my version dropped the symbol for any value under one while keeping it for the rest of the column. The `currency_context` branch of `format_column` now resolves the small currency against that row's context as well. Two differences from the normal path, both deliberate: - `detected_currency` is passed as `None` and `fallback_to_detected=False`. The small formatter is built from `config.currencyFormat` alone and never receives the query-wide detection, so there is nothing to fall back to. - The no-context branch is unchanged. There, a small `CurrencyFormatter` with AUTO and no row data returns the bare formatted number, which is already what `format_number_with_config` does for an unresolved AUTO. Covered by `test_table_small_number_format_resolves_auto_currency_per_row`, which mixes a value above one with three below it across a valid, a whitespace-padded, a null and an invalid currency code. It fails on the previous commit. -- 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]
