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

[MLv2] Add `metabase.lib.binning` and add binning support for fields (#30299)

No `metabase.lib.js` wrapping yet.
parent e6374476
No related branches found
No related tags found
No related merge requests found
......@@ -8,19 +8,39 @@
(comment metabase.shared.util.i18n/keep-me
ttag/keep-me)
(defn- escape-format-string
"Converts `''` to `'` inside the string; that's `java.text.MessageFormat` escaping that isn't needed in JS."
[format-string]
(str/replace format-string #"''" "'"))
(defn js-i18n
"Format an i18n `format-string` with `args` with a translated string in the user locale."
"Format an i18n `format-string` with `args` with a translated string in the user locale.
The strings are formatted in `java.test.MessageFormat` style. That's used directly in JVM Clojure, but in CLJS we have
to adapt to ttag, which doesn't have the same escaping rules.
- 'xyz' single quotes wrap literal text which should not be interpolated, and could contain literal '{0}'.
- A literal single quote is written with two single quotes: `''`
The first part is not supported at all. `''` is converted to a single `'`."
[format-string & args]
(let [strings (str/split format-string #"\{\d+\}")]
(let [strings (-> format-string
escape-format-string
(str/split #"\{\d+\}"))]
(apply ttag/t (clj->js strings) (clj->js args))))
(def ^:private re-param-zero #"\{0\}")
(defn js-i18n-n
"Format an i18n `format-string` with the appropritae plural form based on the value `n`.
"Format an i18n `format-string` with the appropriate plural form based on the value `n`.
Allows `n` to be interpolated into the string using {0}."
[format-string format-string-pl n]
(let [strings (str/split format-string #"\{0\}")
strings (if (= (count strings) 1) [format-string ""] strings)
has-n? (re-find #".*\{0\}.*" format-string)]
(let [format-string-esc (escape-format-string format-string)
strings (str/split format-string-esc re-param-zero)
strings (if (= (count strings) 1)
[format-string-esc ""]
strings)
has-n? (re-find #".*\{0\}.*" format-string-esc)]
(ttag/ngettext (ttag/msgid (clj->js strings) (if has-n? n ""))
format-string-pl
(-> format-string-pl
escape-format-string
(str/replace re-param-zero (str n)))
n)))
(ns metabase.shared.i18n-test
(:require
[clojure.test :refer [are deftest is testing]]
[metabase.shared.util.i18n :as i18n]))
(deftest ^:parallel tru-test
(testing "basic strings"
(is (= "some text here" (i18n/tru "some text here"))))
(testing "escaping single quotes"
(is (= "Where there's life there's hope, and need of vittles."
(i18n/tru "Where there''s life there''s hope, and need of vittles.")))))
(deftest ^:parallel trun-test
(testing "basic"
(are [n exp] (= exp (i18n/trun "{0} cat" "{0} cats" n))
0 "0 cats"
1 "1 cat"
7 "7 cats"))
(testing "escaping in singular"
(are [n exp] (= exp (i18n/trun "{0} cat''s food" "{0} cats worth of food" n))
0 "0 cats worth of food"
1 "1 cat's food"
7 "7 cats worth of food"))
(testing "escaping in both"
(are [n exp] (= exp (i18n/trun "{0} cat''s food" "{0} cats'' food" n))
0 "0 cats' food"
1 "1 cat's food"
7 "7 cats' food")))
......@@ -270,6 +270,7 @@
(defn- supports-numeric-binning? [db]
(and db (driver/database-supports? (:engine db) :binning db)))
;; TODO: Remove all this when the FE is fully ported to [[metabase.lib.binning/available-binning-strategies]].
(defn- assoc-field-dimension-options [{:keys [base_type semantic_type fingerprint] :as field} db]
(let [{min_value :min, max_value :max} (get-in fingerprint [:type :type/Number])
[default-option all-options] (cond
......
(ns metabase.lib.binning
(:require
[metabase.lib.dispatch :as lib.dispatch]
[metabase.lib.hierarchy :as lib.hierarchy]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.binning :as lib.schema.binning]
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
(defmulti with-binning-method
"Implementation for [[with-binning]]. Implement this to tell [[with-binning]] how to add binning to a particular MBQL
clause."
{:arglists '([x binning])}
(fn [x _binning]
(lib.dispatch/dispatch-value x)) :hierarchy lib.hierarchy/hierarchy)
(defmethod with-binning-method :dispatch-type/fn
[f binning]
(fn [query stage-number]
(let [x (f query stage-number)]
(with-binning-method x binning))))
(mu/defn with-binning
"Add binning to an MBQL clause or something that can be converted to an MBQL clause.
Eg. for a Field or Field metadata or `:field` clause, this might do something like this:
(with-binning some-field (bin-by :num-bins 4))
=>
[:field {:binning {:strategy :num-bins :num-bins 4}} 1]
Pass `nil` `binning` to remove any binning."
[x binning :- [:maybe [:or ::lib.schema.binning/binning ::lib.schema.binning/binning-option]]]
(with-binning-method x (if (contains? binning :mbql)
(:mbql binning)
binning)))
(defmulti binning-method
"Implementation of [[binning]]. Return the current binning options associated with `x`."
{:arglists '([x])}
lib.dispatch/dispatch-value
:hierarchy lib.hierarchy/hierarchy)
(defmethod binning-method :default
[_x]
nil)
(mu/defn binning :- [:maybe ::lib.schema.binning/binning]
"Get the current binning options associated with `x`, if any."
[x]
(binning-method x))
(defmulti available-binning-strategies-method
"Implementation for [[available-binning-strategies]]. Return a set of binning strategies from
`:metabase.lib.schema.binning/binning-strategies` that are allowed to be used with `x`."
{:arglists '([query stage-number x])}
(fn [_query _stage-number x]
(lib.dispatch/dispatch-value x))
:hierarchy lib.hierarchy/hierarchy)
(defmethod available-binning-strategies-method :default
[_query _stage-number _x]
nil)
(mu/defn available-binning-strategies :- [:sequential [:ref ::lib.schema.binning/binning-option]]
"Get a set of available binning strategies for `x`. Returns nil if none are available."
([query x]
(available-binning-strategies query -1 x))
([query :- ::lib.schema/query
stage-number :- :int
x]
(available-binning-strategies-method query stage-number x)))
(defn- default-auto-bin []
{:display-name (i18n/tru "Auto bin")
:default true
:mbql {:strategy :default}})
(defn- dont-bin []
{:display-name (i18n/tru "Don''t bin")
:mbql nil})
(defn- with-binning-option-type [m]
(assoc m :lib/type ::binning-option))
(def ^:private *numeric-binning-strategies
(delay (mapv with-binning-option-type
[(default-auto-bin)
{:display-name (i18n/tru "10 bins") :mbql {:strategy :num-bins :num-bins 10}}
{:display-name (i18n/tru "50 bins") :mbql {:strategy :num-bins :num-bins 50}}
{:display-name (i18n/tru "100 bins") :mbql {:strategy :num-bins :num-bins 100}}
(dont-bin)])))
(defn numeric-binning-strategies
"List of binning options for numeric fields. These split the data evenly into a fixed number of bins."
[]
@*numeric-binning-strategies)
(def ^:private *coordinate-binning-strategies
(delay
(mapv with-binning-option-type
[(default-auto-bin)
{:display-name (i18n/tru "Bin every 0.1 degrees") :mbql {:strategy :bin-width :bin-width 0.1}}
{:display-name (i18n/tru "Bin every 1 degree") :mbql {:strategy :bin-width :bin-width 1.0}}
{:display-name (i18n/tru "Bin every 10 degrees") :mbql {:strategy :bin-width :bin-width 10.0}}
{:display-name (i18n/tru "Bin every 20 degrees") :mbql {:strategy :bin-width :bin-width 20.0}}
(dont-bin)])))
(defn coordinate-binning-strategies
"List of binning options for coordinate fields (ie. latitude and longitude). These split the data into as many
ranges as necessary, with each range being a certain number of degrees wide."
[]
@*coordinate-binning-strategies)
(defmethod lib.metadata.calculation/display-info-method ::binning-option
[_query _stage-number binning-option]
(select-keys binning-option [:display-name :default]))
(defn binning-display-name
"This is implemented outside of [[lib.metadata.calculation/display-name]] because it needs access to the field type.
It's called directly by `:field` or `:metadata/field`'s [[lib.metadata.calculation/display-name]]."
[{:keys [bin-width num-bins strategy] :as binning-options} field-metadata]
(when binning-options
(case strategy
:num-bins (i18n/trun "{0} bin" "{0} bins" num-bins)
:bin-width (str bin-width (when (isa? (:semantic-type field-metadata) :type/Coordinate)
"°"))
:default (i18n/tru "Auto binned"))))
......@@ -5,6 +5,7 @@
+ - * / time abs concat replace ref var])
(:require
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.binning :as lib.binning]
[metabase.lib.breakout :as lib.breakout]
[metabase.lib.card :as lib.card]
[metabase.lib.column-group :as lib.column-group]
......@@ -29,6 +30,7 @@
[metabase.shared.util.namespaces :as shared.ns]))
(comment lib.aggregation/keep-me
lib.binning/keep-me
lib.breakout/keep-me
lib.card/keep-me
lib.column-group/keep-me
......@@ -67,6 +69,10 @@
sum
sum-where
var]
[lib.binning
available-binning-strategies
binning
with-binning]
[lib.breakout
breakout
breakoutable-columns
......
......@@ -2,11 +2,13 @@
(:require
[medley.core :as m]
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.binning :as lib.binning]
[metabase.lib.expression :as lib.expression]
[metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.normalize :as lib.normalize]
[metabase.lib.options :as lib.options]
[metabase.lib.ref :as lib.ref]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.common :as lib.schema.common]
......@@ -17,6 +19,7 @@
[metabase.lib.temporal-bucket :as lib.temporal-bucket]
[metabase.lib.util :as lib.util]
[metabase.shared.util.i18n :as i18n]
[metabase.util :as u]
[metabase.util.humanization :as u.humanization]
[metabase.util.log :as log]
[metabase.util.malli :as mu]))
......@@ -145,7 +148,9 @@
;;; TODO -- base type should be affected by `temporal-unit`, right?
(defmethod lib.metadata.calculation/metadata-method :field
[query stage-number [_tag {:keys [source-field effective-type base-type temporal-unit join-alias], :as opts} :as field-ref]]
[query
stage-number
[_tag {:keys [base-type binning effective-type join-alias source-field temporal-unit], :as opts} :as field-ref]]
(let [field-metadata (resolve-field-metadata query stage-number field-ref)
metadata (merge
{:lib/type :metadata/field}
......@@ -158,6 +163,8 @@
{:base-type base-type})
(when temporal-unit
{::temporal-unit temporal-unit})
(when binning
{::binning binning})
(when join-alias
{::join-alias join-alias})
(when source-field
......@@ -171,10 +178,11 @@
[query stage-number {field-display-name :display-name
field-name :name
temporal-unit :unit
binning :binning
join-alias :source_alias
fk-field-id :fk-field-id
table-id :table-id
:as _field-metadata} style]
:as field-metadata} style]
(let [field-display-name (or field-display-name
(u.humanization/name->human-readable-name :simple field-name))
join-display-name (when (= style :long)
......@@ -188,18 +196,20 @@
display-name (if join-display-name
(str join-display-name " → " field-display-name)
field-display-name)]
(if temporal-unit
(lib.util/format "%s (%s)" display-name (name temporal-unit))
display-name)))
(cond
temporal-unit (lib.util/format "%s (%s)" display-name (name temporal-unit))
binning (lib.util/format "%s: %s" display-name (lib.binning/binning-display-name binning field-metadata))
:else display-name)))
(defmethod lib.metadata.calculation/display-name-method :field
[query
stage-number
[_tag {:keys [join-alias temporal-unit source-field], :as _opts} _id-or-name, :as field-clause]
[_tag {:keys [binning join-alias temporal-unit source-field], :as _opts} _id-or-name, :as field-clause]
style]
(if-let [field-metadata (cond-> (resolve-field-metadata query stage-number field-clause)
join-alias (assoc :source_alias join-alias)
temporal-unit (assoc :unit temporal-unit)
binning (assoc :binning binning)
source-field (assoc :fk-field-id source-field))]
(lib.metadata.calculation/display-name query stage-number field-metadata style)
;; mostly for the benefit of JS, which does not enforce the Malli schemas.
......@@ -227,6 +237,7 @@
(when-let [card (lib.metadata/card query card-id)]
{:table {:name (:name card), :display-name (:name card)}})))))
;;; ---------------------------------- Temporal Bucketing ----------------------------------------
(defmethod lib.temporal-bucket/temporal-bucket-method :field
[[_tag opts _id-or-name]]
(:temporal-unit opts))
......@@ -281,6 +292,43 @@
(isa? effective-type :type/Time) lib.temporal-bucket/time-bucket-options
:else [])))
;;; ---------------------------------------- Binning ---------------------------------------------
(defmethod lib.binning/binning-method :field
[field-clause]
(-> field-clause lib.options/options :binning))
(defmethod lib.binning/binning-method :metadata/field
[metadata]
(::binning metadata))
(defmethod lib.binning/with-binning-method :field
[field-clause binning]
(lib.options/update-options field-clause u/assoc-dissoc :binning binning))
(defmethod lib.binning/with-binning-method :metadata/field
[metadata binning]
(u/assoc-dissoc metadata ::binning binning))
(defmethod lib.binning/available-binning-strategies-method :field
[query stage-number field-ref]
(lib.binning/available-binning-strategies query stage-number (resolve-field-metadata query stage-number field-ref)))
(defmethod lib.binning/available-binning-strategies-method :metadata/field
[query _stage-number {:keys [effective-type fingerprint semantic-type] :as _field-metadata}]
(let [binning? (some-> query lib.metadata/database :features (contains? :binning))
{min-value :min max-value :max} (get-in fingerprint [:type :type/Number])]
(cond
;; TODO: Include the time and date binning strategies too; see metabase.api.table/assoc-field-dimension-options.
(and binning? min-value max-value
(isa? semantic-type :type/Coordinate))
(lib.binning/coordinate-binning-strategies)
(and binning? min-value max-value
(isa? effective-type :type/Number)
(not (isa? semantic-type :Relation/*)))
(lib.binning/numeric-binning-strategies))))
;;; -------------------------------------- Join Alias --------------------------------------------
(defmethod lib.join/current-join-alias-method :field
[[_tag opts]]
(get opts :join-alias))
......@@ -316,10 +364,10 @@
{:join-alias join-alias})
(when-let [temporal-unit (::temporal-unit metadata)]
{:temporal-unit temporal-unit})
(when-let [binning (::binning metadata)]
{:binning binning})
(when-let [source-field-id (:fk-field-id metadata)]
{:source-field source-field-id})
;; TODO -- binning options.
)
{:source-field source-field-id}))
always-use-name? (#{:source/card :source/native :source/previous-stage} (:lib/source metadata))]
[:field options (if always-use-name?
(:name metadata)
......
(ns metabase.lib.options
(:require
[metabase.shared.util.i18n :as i18n]
[metabase.util :as u]
[metabase.util.malli :as mu]))
;;; TODO -- not 100% sure we actually need all of this stuff anymore.
......@@ -40,7 +41,9 @@
(mu/defn with-options
"Update `x` so its [[options]] are `new-options`. If the clause or map already has options, this will
*replace* the old options; if it does not, this will the new options.
*replace* the old options; if it does not, this will set the new options.
If `x` is a map with `:lib/options` and `new-options` is `empty?`, this will drop `:lib/options` entirely.
You should probably prefer [[update-options]] to using this directly, so you don't stomp over existing stuff
unintentionally. Implement this if you need to teach Metabase lib how to support something that doesn't follow the
......@@ -48,7 +51,7 @@
[x new-options :- [:maybe map?]]
(cond
(map? x)
(assoc x :lib/options new-options)
(u/assoc-dissoc x :lib/options (not-empty new-options))
(mbql-clause? x)
(if ((some-fn nil? map?) (second x))
......
(ns metabase.lib.schema.binning
"Malli schema for binning of a column's values.
There are two approaches to binning, selected by `:strategy`:
- `{:strategy :bin-width :bin-width 10}` makes 1 or more bins that are 10 wide;
- `{:strategy :num-bins :num-bins 12}` splits the column into 12 bins."
(:require
[metabase.lib.schema.common :as lib.schema.common]
[metabase.util.malli.registry :as mr]))
(mr/def ::binning-strategies
[:enum :bin-width :default :num-bins])
(mr/def ::binning
[:and
[:map
[:strategy [:ref ::binning-strategies]]
[:bin-width {:optional true} pos?]
[:num-bins {:optional true} ::lib.schema.common/int-greater-than-zero]]
[:fn {:error/message "if :strategy is not :default, the matching key :bin-width or :num-bins must also be set"}
#(when-let [strat (:strategy %)]
(or (= strat :default)
(contains? % strat)))]])
(mr/def ::binning-option
[:map
[:lib/type [:= :metabase.lib.binning/binning-option]]
[:display-name :string]
[:mbql [:maybe ::binning]]
[:default {:optional true} :boolean]])
......@@ -773,3 +773,14 @@
(regexp? x) :dispatch-type/regex
;; we should add more mappings here as needed
:else :dispatch-type/*))
(defn assoc-dissoc
"Called like `(assoc m k v)`, this does [[assoc]] if `(some? v)`, and [[dissoc]] if not.
Put another way: `k` will either be set to `v`, or removed.
Note that if `v` is `false`, it will be handled with [[assoc]]; only `nil` causes a [[dissoc]]."
[m k v]
(if (some? v)
(assoc m k v)
(dissoc m k)))
......@@ -2,6 +2,7 @@
(:require
[clojure.test :refer [are deftest is testing]]
[medley.core :as m]
[metabase.lib.binning :as lib.binning]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
......@@ -228,6 +229,80 @@
(lib/available-temporal-buckets (:query temporal-bucketing-mock-metadata)
(lib/with-temporal-bucket x :month-of-year))))))))))
(deftest ^:parallel unresolved-lib-field-with-binning-test
(let [query (lib/query-for-table-name meta/metadata-provider "ORDERS")
binning {:strategy :num-bins
:num-bins 10}
f (lib/with-binning (lib/field (meta/id :orders :subtotal)) binning)]
(is (fn? f))
(let [field (f query -1)]
(is (=? [:field {:binning binning} (meta/id :orders :subtotal)]
field))
(testing "(lib/binning <column-metadata>)"
(is (= binning
(lib/binning (lib.metadata.calculation/metadata query -1 field)))))
(testing "(lib/binning <field-ref>)"
(is (= binning
(lib/binning field))))
#?(:clj
;; i18n/trun doesn't work in the CLJS tests, only in proper FE, so this test is JVM-only.
(is (= "Subtotal: 10 bins"
(lib.metadata.calculation/display-name query -1 field)))))))
(deftest ^:parallel with-binning-test
(doseq [[binning1 binning2] (partition 2 1 [{:strategy :default}
{:strategy :num-bins :num-bins 10}
{:strategy :bin-width :bin-width 1.0}
{:strategy :default}])
:let [field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")]
[what x] {"column metadata" field-metadata
"field ref" (lib/ref field-metadata)}
:let [x' (lib/with-binning x binning1)]]
(testing (str what " strategy = " (:strategy binning2) "\n\n" (u/pprint-to-str x') "\n")
(testing "lib/binning should return the binning settings"
(is (= binning1
(lib/binning x'))))
(testing "should generate a :field ref with correct :binning"
(is (=? [:field
{:lib/uuid string?
:binning binning1}
integer?]
(lib/ref x'))))
(testing "remove the binning setting"
(let [x'' (lib/with-binning x' nil)]
(is (nil? (lib/binning x'')))
(is (= x
x''))))
(testing "change the binning setting, THEN remove it"
(let [x'' (lib/with-binning x' binning2)
x''' (lib/with-binning x'' nil)]
(is (= binning2
(lib/binning x'')))
(is (nil? (lib/binning x''')))
(is (= x
x''')))))))
(deftest ^:parallel available-binning-strategies-test
(doseq [{:keys [expected-options field-metadata query]}
[{:query (lib/query-for-table-name meta/metadata-provider "ORDERS")
:field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")
:expected-options (lib.binning/numeric-binning-strategies)}
{:query (lib/query-for-table-name meta/metadata-provider "PEOPLE")
:field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "PEOPLE" "LATITUDE")
:expected-options (lib.binning/coordinate-binning-strategies)}]]
(testing (str (:semantic-type field-metadata) " Field")
(doseq [[what x] [["column metadata" field-metadata]
["field ref" (lib/ref field-metadata)]]]
(testing (str what "\n\n" (u/pprint-to-str x))
(is (= expected-options
(lib/available-binning-strategies query x)))
(testing "when binned, should still return the same available units"
(let [binned (lib/with-binning x (second expected-options))]
(is (= (-> expected-options second :mbql)
(lib/binning binned)))
(is (= expected-options
(lib/available-binning-strategies query binned))))))))))
(deftest ^:parallel joined-field-column-name-test
(let [card {:dataset-query {:database (meta/id)
:type :query
......
......@@ -381,3 +381,13 @@
:dispatch-type/regex
:dispatch-type/fn
:dispatch-type/*)))
(deftest ^:parallel assoc-dissoc-test
(testing `lib.options/with-option-value
(is (= {:foo "baz"}
(u/assoc-dissoc {:foo "bar"} :foo "baz")))
(is (= {}
(u/assoc-dissoc {:foo "bar"} :foo nil)))
(is (= {:foo false}
(u/assoc-dissoc {:foo "bar"} :foo false))
"false should be assoc'd")))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment