Skip to content
Snippets Groups Projects
Unverified Commit e7e422cf authored by Braden Shepherdson's avatar Braden Shepherdson Committed by GitHub
Browse files

[metabase-lib] Number and currency formatting polish (#27616) 

- Use currency-specific decimals count (eg. 0 for JPY, 8 for BTC)
- Use plural currency names in Java, matching JS
    - previously: singular proper name "7.23 US Dollar"
    - now: plural prose "7.23 US dollars"
- "Small numbers" hack to treat min/max fraction digits as significant
  digits now applies to percentages under 1%, as well as the original
  |n| < 1
parent f395e2f2
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 104 additions and 74 deletions
frontend/src/metabase/visualizations/lib/chart_values.js
+ 2
− 1
View file @ e7e422cf
import _ from "underscore";
import { compact_currency_options_js } from "cljs/metabase.shared.formatting.numbers";
import { moveToFront } from "metabase/lib/dom";
import { isHistogramBar, xValueForWaterfallTotal } from "./renderer_utils";
......@@ -183,7 +184,7 @@ export function onRenderValueLabels(
// those settings.
minimum_fraction_digits: 2,
maximum_fraction_digits: 2,
currency_style: "symbol",
...compact_currency_options_js,
};
const lengths = data.map(d => formatYValue(d.y, options).length);
return lengths.reduce((sum, l) => sum + l, 0) / lengths.length;
......
frontend/test/metabase/lib/formatting.unit.spec.js
+ 1
− 1
View file @ e7e422cf
......@@ -105,7 +105,7 @@ describe("formatting", () => {
it("from positive JPY", () => {
expect(formatNumber(1234.56, { ...options, currency: "JPY" })).toBe(
"1,234.56",
"1,235",
);
});
......
frontend/test/metabase/scenarios/question/settings.cy.spec.js
+ 2
− 2
View file @ e7e422cf
......@@ -231,8 +231,8 @@ describe("scenarios > question > settings", () => {
cy.findByText("In every table cell").click();
cy.findByText("₿2.07");
cy.findByText("₿6.10");
cy.findByText("₿2.07000000");
cy.findByText("₿6.10000000");
});
});
......
shared/src/metabase/shared/formatting/internal/numbers.clj
+ 29
− 23
View file @ e7e422cf
......@@ -27,14 +27,15 @@
(Currency/getInstance)
(.getSymbol locale))))
(defn- apply-currency-style [text ^Currency currency ^Locale locale style currency-key]
(defn- apply-currency-style [text ^Currency _currency ^Locale locale style currency-key]
(let [sym (symbol-for currency-key locale)
label (if (= currency-key :BTC) "Bitcoin" (.getDisplayName currency locale))
code (if (= currency-key :BTC) "BTC" (.getCurrencyCode currency))]
;; TODO Our currency table has plurals but no translation; Java's `Currency.getDisplayName` is singular but
;; translated. We should get the names in currency/currency keyed by locale.
currency (get currency/currency currency-key)]
(case (or style "symbol")
"symbol" text ; The default, already handled.
"name" (str (str/replace text sym "") " " label)
"code" (str/replace text sym (str code core/non-breaking-space)))))
"symbol" (str/replace text sym (:symbol currency)) ; Java's symbols are not identical to ours
"name" (str (str/replace text sym "") " " (:name_plural currency))
"code" (str/replace text sym (str (:code currency) core/non-breaking-space)))))
;; Core internals =================================================================================================
(def ^:private bad-currencies
......@@ -47,18 +48,22 @@
(Locale. (:locale options))
(Locale/getDefault)))
(defn- number-formatter-for-options-baseline ^NumberFormat [options locale]
(case (:number-style options)
;; For scientific, assemble the 0.###E0 DecimalFormat pattern.
"scientific" (DecimalFormat. (str "0."
(str-run (:minimum-fraction-digits options 0) "0")
(str-run (- (:maximum-fraction-digits options 2)
(:minimum-fraction-digits options 0))
"#")
"E0"))
"currency" (NumberFormat/getCurrencyInstance locale)
(doto (NumberFormat/getInstance locale)
(.setMaximumFractionDigits (:maximum-fraction-digits options 300)))))
(defn- number-formatter-for-options-baseline
^NumberFormat [{:keys [maximum-fraction-digits minimum-fraction-digits number-style]} locale]
(let [^NumberFormat nf (case number-style
;; For scientific, assemble the 0.###E0 DecimalFormat pattern.
"scientific" (DecimalFormat. (str "0."
(str-run (or minimum-fraction-digits 0) "0")
(str-run (- (or maximum-fraction-digits 2)
(or minimum-fraction-digits 0))
"#")
"E0"))
"currency" (NumberFormat/getCurrencyInstance locale)
"percent" (NumberFormat/getPercentInstance locale)
(NumberFormat/getInstance locale))]
(when (not (= number-style #"scientific"))
(.setMaximumFractionDigits nf (or maximum-fraction-digits 300)))
nf))
(defn- set-rounding! [^NumberFormat nf]
;; JavaScript does not support picking the rounding mode; it's always HALF_UP.
......@@ -78,11 +83,12 @@
(Currency/getInstance (name currency))))))
(defn- set-separators! [^NumberFormat nf options]
(when (:number-separators options)
(when-let [[decimal grouping] (:number-separators options)]
(let [^DecimalFormat df nf
syms (doto (.getDecimalFormatSymbols df)
(.setDecimalSeparator (first (:number-separators options))))]
(if-let [grouping (second (:number-separators options))]
syms (.getDecimalFormatSymbols df)]
(when decimal
(.setDecimalSeparator syms decimal))
(if grouping
(.setGroupingSeparator syms grouping)
(.setGroupingUsed df false))
(.setDecimalFormatSymbols df syms))))
......@@ -120,7 +126,7 @@
(and currency (bad-currencies currency))
(attach-currency-symbol nf locale currency)
;; Handle the :currency-style option, which isn't supported natively on Java.
(and currency (not= (:currency-style options) "symbol"))
currency
(apply-currency-style (.getCurrency nf) locale (:currency-style options) currency)))
(wrap-currency [_ text]
......
shared/src/metabase/shared/formatting/internal/numbers_core.cljc
+ 13
− 3
View file @ e7e422cf
(ns metabase.shared.formatting.internal.numbers-core
"Cross-platform foundation for the number formatters.")
"Cross-platform foundation for the number formatters."
(:require
[metabase.shared.util.currency :as currency]))
;; Options ========================================================================================================
(defn- default-decimal-places [{:keys [currency number-style]}]
(if (and currency (= number-style "currency"))
(let [places (-> currency keyword (@currency/currency-map) :decimal_digits)]
{:minimum-fraction-digits places
:maximum-fraction-digits places})
{:maximum-fraction-digits 2}))
(defn prep-options
"Transforms input options with defaults and other adjustments.
Defaults:
- :maximum-fraction-digits 2 if not specified
- `:maximum-fraction-digits` is 2 if not specified
- BUT if `:currency` is set, `:minimum-fraction-digits = :maximum-fraction-digits = (:decimal_digits currency)`
Adjustments:
- :decimals is dropped, and both min and max fraction-digits are set to that value."
......@@ -15,7 +25,7 @@
(dissoc :decimals)
(assoc :maximum-fraction-digits (:decimals options)
:minimum-fraction-digits (:decimals options))))]
(cond-> (merge {:maximum-fraction-digits 2} options)
(cond-> (merge (default-decimal-places options) options)
(:decimals options) expand-decimals)))
(def non-breaking-space
......
shared/src/metabase/shared/formatting/numbers.cljc
+ 39
− 24
View file @ e7e422cf
......@@ -6,9 +6,15 @@
(declare format-number)
(def ^:private compact-currency-options
(def compact-currency-options
"Extra defaults that are mixed in when formatted a currency value in compact mode."
{:currency-style "symbol"})
#?(:cljs
(def ^:export compact-currency-options-js
"Extra defaults that are mixed in when formatted a currency value in compact mode."
(clj->js compact-currency-options)))
;; Compact form ===================================================================================================
(def ^:private display-compact-decimals-cutoff 1000)
......@@ -18,31 +24,39 @@
[1000000 "M"]
[1000 "k"]])
(defn- format-number-compact-basic [number]
(let [abs-value (abs number)]
(defn- format-number-compact-basic [number options]
(let [options (dissoc options :compact :number-style)
abs-value (abs number)]
(cond
(zero? number) "0"
(< abs-value display-compact-decimals-cutoff) (format-number number {})
(< abs-value display-compact-decimals-cutoff) (format-number number options)
:else (let [[power suffix] (first (filter #(>= abs-value (first %)) humanized-powers))]
(str (format-number (/ number power) {:minimum-fraction-digits 1 :maximum-fraction-digits 1})
(str (format-number (/ number power)
(merge options {:minimum-fraction-digits 1 :maximum-fraction-digits 1}))
suffix)))))
(defmulti ^:private format-number-compact (fn [_ {:keys [number-style]}] number-style))
(defmulti ^:private format-number-compact* (fn [_ {:keys [number-style]}] number-style))
(defmethod format-number-compact :default [number _options]
(format-number-compact-basic number))
(defmethod format-number-compact* :default [number options]
(format-number-compact-basic number options))
(defmethod format-number-compact "percent" [number _options]
(str (format-number-compact-basic (* 100 number)) "%"))
(defmethod format-number-compact* "percent" [number options]
(str (format-number-compact-basic (* 100 number) options) "%"))
(defmethod format-number-compact "currency" [number options]
(let [formatter (internal/number-formatter-for-options (merge options compact-currency-options))]
(defmethod format-number-compact* "currency" [number options]
(let [options (merge options compact-currency-options)
formatter (internal/number-formatter-for-options options)]
(if (< (abs number) display-compact-decimals-cutoff)
(core/format-number-basic formatter number)
(core/wrap-currency formatter (format-number-compact-basic number)))))
(core/wrap-currency formatter (format-number-compact-basic number options)))))
(defmethod format-number-compact* "scientific" [number options]
(internal/format-number-scientific number (merge options {:maximum-fraction-digits 1 :minimum-fraction-digits 1})))
(defmethod format-number-compact "scientific" [number options]
(internal/format-number-scientific number (merge {:maximum-fraction-digits 1 :minimum-fraction-digits 1} options)))
(defn- format-number-compact [number options]
(format-number-compact* number (-> options
(dissoc :compact)
core/prep-options)))
;; High-level =====================================================================================================
(defn- format-number-standard [number options]
......@@ -52,10 +66,14 @@
;; Hacky special case inherited from the TS version - to match classic behavior for small numbers,
;; treat maximum-fraction-digits as maximum-significant-digits instead.
(and (< (abs number) 1)
(not (:decimals options))
;; "Small" means |x| < 1, or < 1% for percentages.
(and (not (:decimals options))
(not (:minimum-fraction-digits options))
(not (#{"percent" "currency"} (:number-style options))))
(not= (:number-style options) "currency")
(< (abs number)
(if (= (:number-style options) "percent")
0.01
1)))
(-> options
(dissoc :maximum-fraction-digits)
(assoc :maximum-significant-digits (max 2 (:minimum-significant-digits options 0)))
......@@ -78,9 +96,9 @@
- `:negative-in-parentheses` boolean: True wraps negative values in parentheses; false (the default) uses minus signs.
- `:number-serpators` string: A two-character string \"ab\" where `a` is the decimal symbol and `b` is the grouping.
Default is American-style \".,\".
- `:number-style` \"currency\" | \"decimal\" | \"scientific\" | \"percentage\": The fundamental type to display.
- `:number-style` \"currency\" | \"decimal\" | \"scientific\" | \"percent\": The fundamental type to display.
- \"currency\" renders as eg. \"$123.45\" based on the `:currency` value.
- \"percentage\" renders eg. 0.432 as \"43.2%\".
- \"percent\" renders eg. 0.432 as \"43.2%\".
- \"scientific\" renders in scientific notation with 1 integer digit: eg. 0.00432 as \"4.32e-3\".
- \"decimal\" (the default) is basic numeric notation.
- `:scale` number: Gives a factor by which to multiply the value before rendering it."
......@@ -94,10 +112,7 @@
(format-number (- number) (assoc options :negative-in-parentheses false))
")")
compact (format-number-compact number options)
compact (format-number-compact number options)
(= (keyword number-style)
:scientific) (internal/format-number-scientific number options)
:else (format-number-standard number options))))
;; TODO(braden) We have recommended decimal places for currencies (eg. 2 for USD and EUR, 0 for JPY, 6 for BTC) in our
;; currency.cljc table. We should use that for formatting (non-compact) currencies, unless the input overrides.
shared/test/metabase/shared/formatting/numbers_test.cljc
+ 18
− 20
View file @ e7e422cf
......@@ -3,7 +3,7 @@
[clojure.test :refer [are deftest is testing]]
[metabase.shared.formatting.numbers :as numbers]))
(deftest basics-test
(deftest ^:parallel basics-test
(testing "format-number basics"
(are [s n] (= s (numbers/format-number n {}))
"0" 0
......@@ -53,13 +53,13 @@
"1.01" 1.011
"1.01" 1.0111)))
(deftest negative-in-parentheses-test
(deftest ^:parallel negative-in-parentheses-test
(are [s n] (= s (numbers/format-number n {:negative-in-parentheses true}))
"7" 7
"(4)" -4
"0" 0))
(deftest compact-mode-test
(deftest ^:parallel compact-mode-test
(testing "zero"
(is (= "0" (numbers/format-number 0 {:compact true}))))
......@@ -161,7 +161,7 @@
"$-1.2M" -1234567.89 "USD"
"CN¥1.2M" 1234567.89 "CNY")))
(deftest currency-test
(deftest ^:parallel currency-test
(are [s n c] (= s (numbers/format-number n {:number-style "currency" :currency c :locale "en"}))
"$1.23" 1.23 "USD"
"-$1.23" -1.23 "USD"
......@@ -176,19 +176,17 @@
"$1,234.56" 1234.56 "USD"
"$1,234,567.89" 1234567.89 "USD"
"-$1,234,567.89" -1234567.89 "USD"
"₿6.35" 6.34527 "BTC" ;; Tests fix-currency-symbols logic in CLJS, Intl returns "BTC" by default.
"CN¥1,234,567.89" 1234567.89 "CNY"
"¥1,234,567.89" 1234567.89 "JPY")
"₿6.34527873" 6.345278729 "BTC" ;; BTC is not natively supported, but we fix the symbols. 8 decimal places!
"¥1,234,568" 1234567.89 "JPY" ;; 0 decimal places for JPY.
"CN¥1,234,567.89" 1234567.89 "CNY")
(testing "by name"
(let [labels #?(:clj {"USD" "US Dollar"
"BTC" "Bitcoin"
"CNY" "Chinese Yuan"
"JPY" "Japanese Yen"}
:cljs {"USD" "US dollars"
"BTC" "BTC"
"CNY" "Chinese yuan"
"JPY" "Japanese yen"})]
(let [labels {"USD" "US dollars"
;; TODO Override this in the JS side? This is the only spot where the names from Intl.NumberFormat and
;; metabase.shared.util.currency differ.
"BTC" #?(:clj "Bitcoins" :cljs "BTC")
"CNY" "Chinese yuan"
"JPY" "Japanese yen"}]
(are [s n c] (= (str s " " (get labels c))
(numbers/format-number n {:number-style "currency" :currency c :currency-style "name"}))
"1.23" 1.23 "USD"
......@@ -204,9 +202,9 @@
"1,234.56" 1234.56 "USD"
"1,234,567.89" 1234567.89 "USD"
"-1,234,567.89" -1234567.89 "USD"
"6.35" 6.34527 "BTC"
"6.34527298" 6.345272982 "BTC"
"1,234,567.89" 1234567.89 "CNY"
"1,234,567.89" 1234567.89 "JPY")))
"1,234,568" 1234567.89 "JPY")))
(testing "by code"
(are [s n c] (= s (numbers/format-number n {:number-style "currency" :currency c :currency-style "code"}))
......@@ -224,11 +222,11 @@
"USD\u00a01,234.56" 1234.56 "USD"
"USD\u00a01,234,567.89" 1234567.89 "USD"
"-USD\u00a01,234,567.89" -1234567.89 "USD"
"BTC\u00a06.35" 6.34527 "BTC"
"BTC\u00a06.34527192" 6.345271916 "BTC"
"CNY\u00a01,234,567.89" 1234567.89 "CNY"
"JPY\u00a01,234,567.89" 1234567.89 "JPY")))
"JPY\u00a01,234,568" 1234567.89 "JPY")))
(deftest scientific-test
(deftest ^:parallel scientific-test
(testing "defaults to 0-2 decimal places if not specified"
(are [s n] (= s (numbers/format-number n {:number-style "scientific"}))
"0e+0" 0
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment