Skip to content
Snippets Groups Projects
Unverified Commit c3f97bbb authored by Howon Lee's avatar Howon Lee Committed by GitHub
Browse files

Reduce fieldname friendliness (#16119) 

We made fieldname friendliness, people didn't like it, we tried to fix it, people still didn't like it, now we're just making the fieldnames all mildly unfriendly again. Not like, snake_case unfriendly, but Pascal Case With Spaces unfriendly
parent 4347464b
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 42 additions and 248 deletions
frontend/src/metabase/admin/settings/selectors.js
+ 2
− 3
View file @ c3f97bbb
......@@ -90,14 +90,13 @@ const SECTIONS = updateSectionsWithPlugins({
display_name: t`Friendly Table and Field Names`,
type: "select",
options: [
{ value: "advanced", name: t`Enabled` },
{
value: "simple",
name: t`Only replace underscores and dashes with spaces`,
name: t`Replace underscores and dashes with spaces`,
},
{ value: "none", name: t`Disabled` },
],
defaultValue: "advanced",
defaultValue: "simple",
},
{
key: "enable-nested-queries",
......
modules/drivers/redshift/test/metabase/driver/redshift_test.clj
+ 2
− 2
View file @ c3f97bbb
......@@ -128,7 +128,7 @@
:parent_id nil
:id (mt/id :extsales :buyerid)
:visibility_type :normal
:display_name "Buyer ID"
:display_name "Buyerid"
:base_type :type/Integer
:effective_type :type/Integer
:coercion_strategy nil}
......@@ -142,7 +142,7 @@
:parent_id nil
:id (mt/id :extsales :salesid)
:visibility_type :normal
:display_name "Sale Sid"
:display_name "Salesid"
:base_type :type/Integer
:effective_type :type/Integer
:coercion_strategy nil}]
......
src/metabase/models/humanization.clj
+ 23
− 37
View file @ c3f97bbb
......@@ -2,28 +2,19 @@
"Logic related to humanization of table names and other identifiers, e.g. taking an identifier like `my_table` and
returning a human-friendly one like `My Table`.
There are currently three implementations of humanization logic; `:advanced`, cost-based logic is the default; which
implementation is used is determined by the Setting `humanization-strategy`; `:simple`, which merely replaces
underscores and dashes with spaces, and `:none`, which predictibly is merely an identity function that does nothing
to the results.
There are currently two implementations of humanization logic, previously three.
Which implementation is used is determined by the Setting `humanization-strategy`.
`:simple`, which merely replaces underscores and dashes with spaces, and `:none`,
which predictibly is merely an identity function that does nothing to the results.
The actual algorithm for advanced humanization is in `metabase.util.infer-spaces`. (NOTE: some of the logic is here,
such as the `captialize-word` function; maybe we should move that so all the logic is in one place?)"
There used to also be `:advanced`, which was the default until enough customers
complained that we first fixed it and then the fix wasn't good enough so we removed it."
(:require [clojure.string :as str]
[clojure.tools.logging :as log]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.infer-spaces :refer [infer-spaces]]
[toucan.db :as db]))
(def ^:private ^:const acronyms
#{"id" "url" "ip" "uid" "uuid" "guid"})
(defn- capitalize-word [word]
(if (contains? acronyms (str/lower-case word))
(str/upper-case word)
(str/capitalize word)))
(declare humanization-strategy)
(defmulti ^String name->human-readable-name
......@@ -31,33 +22,30 @@
strategy defined by the Setting `humanization-strategy`. With two args, you may specify a custom strategy (intended
mainly for the internal implementation):
(humanization-strategy :advanced)
(name->human-readable-name \"cooltoucans\") ;-> \"Cool Toucans\"
(humanization-strategy :simple)
(name->human-readable-name \"cool_toucans\") ;-> \"Cool Toucans\"
;; this is the same as:
(name->human-readable-name (humanization-strategy) \"cooltoucans\") ;-> \"Cool Toucans\"
(name->human-readable-name (humanization-strategy) \"cool_toucans\") ;-> \"Cool Toucans\"
;; specifiy a different strategy:
(name->human-readable-name :none \"cooltoucans\") ;-> \"cooltoucans\""
(name->human-readable-name :none \"cool_toucans\") ;-> \"cool_toucans\""
{:arglists '([s] [strategy s])}
(fn
([_] (keyword (humanization-strategy)))
([strategy _] (keyword strategy))))
;; :advanced is the default implementation; splits words with cost-based fn
(defmethod name->human-readable-name :advanced
([s] (name->human-readable-name :advanced s))
([_, ^String s]
;; explode string on hyphens, underscores, spaces, and camelCase
(when (seq s)
(str/join " " (for [part (str/split s #"[-_\s]+|(?<=[a-z])(?=[A-Z])")
:when (not (str/blank? part))
word (dedupe (flatten (infer-spaces part)))]
(capitalize-word word))))))
(def ^:private ^:const acronyms
#{"id" "url" "ip" "uid" "uuid" "guid"})
;; simple replaces hyphens and underscores with spaces
(defn- capitalize-word [word]
(if (contains? acronyms (str/lower-case word))
(str/upper-case word)
(str/capitalize word)))
;; simple replaces hyphens and underscores with spaces and capitalizes
(defmethod name->human-readable-name :simple
([s] (name->human-readable-name :simple s))
([_, ^String s]
;; explode on hypens, underscores, and spaces
;; explode on hyphens, underscores, and spaces
(when (seq s)
(str/join " " (for [part (str/split s #"[-_\s]+")
:when (not (str/blank? part))]
......@@ -93,7 +81,7 @@
(defn- set-humanization-strategy! [new-value]
(let [new-strategy (or new-value "advanced")]
(let [new-strategy (or new-value "simple")]
;; check to make sure `new-strategy` is a valid strategy, or throw an Exception it is it not.
(when-not (get-method name->human-readable-name (keyword new-strategy))
(throw (IllegalArgumentException.
......@@ -108,10 +96,8 @@
(re-humanize-table-and-field-names! old-strategy))))
(defsetting ^{:added "0.28.0"} humanization-strategy
(str (deferred-tru "Metabase can attempt to transform your table and field names into more sensible, human-readable versions, e.g. \"somehorriblename\" becomes \"Some Horrible Name\".")
" "
(deferred-tru "This doesn’t work all that well if the names are in a language other than English, however.")
(str (deferred-tru "To make table and field names more human-friendly, Metabase will replace dashes and underscores in them with spaces.")
" "
(deferred-tru "Do you want us to take a guess?"))
:default "advanced"
(deferred-tru "We’ll capitalize each word while at it, so ‘last_visited_at’ will become ‘Last Visited At’."))
:default "simple"
:setter set-humanization-strategy!)
src/metabase/util/infer_spaces.clj deleted 100644 → 0
+ 0
− 106
View file @ 4347464b
(ns metabase.util.infer-spaces
"Logic for automatically inferring where spaces should go in table names. Ported from
https://stackoverflow.com/questions/8870261/how-to-split-text-without-spaces-into-list-of-words/11642687#11642687."
;; TODO - The code in this namespace is very hard to understand. We should clean it up and make it readable.
(:require [clojure.java.io :as io]
[clojure.string :as str])
(:import java.lang.Math
java.util.Arrays))
(def ^:const ^:private special-words ["checkins"])
(defn- slurp-words-by-frequency []
(concat special-words (str/split-lines (slurp (io/resource "words-by-inv-frequency.txt")))))
;; wordcost = dict((k, log((i+1)*log(len(words)))) for i,k in enumerate(words))
(defn- make-cost-map
"Creates a map keyed by the hash of the word and the cost as the value. The map is sorted by the hash value"
[words]
(let [log-count (Math/log (count words))]
(into (sorted-map)
(map-indexed (fn [idx word]
[(hash word) (float (Math/log (* (inc idx) log-count)))])
words))))
;; # Build arrays for a cost lookup, assuming Zipf's law and cost = -math.log(probability).
;;
;; This is structured as a let for efficiency reasons. It's reading in 120k strings and putting them into two
;; correlated data structures. We want to ensure that those strings are garbage collected after we setup the
;; structures and we don't want to read the file in twice
(let [all-words (slurp-words-by-frequency)
sorted-words (make-cost-map all-words)]
(def ^:private ^"[I" word-hashes
"Array of word hash values, ordered by that hash value"
(int-array (keys sorted-words)))
(def ^:private ^"[F" word-cost
"Array of word cost floats, ordered by the hash value for that word"
(float-array (vals sorted-words)))
;; maxword = max(len(x) for x in words)
(def ^:private max-word
"Length of the longest word in the word list"
(apply max (map count all-words))))
(defn- get-word-cost
"Finds `S` in the word list. If found, returns the cost, otherwise returns `DEFAULT` like clojure.core's `get`"
[s default]
(let [idx (Arrays/binarySearch word-hashes (int (hash s)))]
;; binarySearch returns a negative number if not found
(if (< idx 0)
default
(aget word-cost idx))))
;; def infer_spaces(s):
;; """Uses dynamic programming to infer the location of spaces in a string
;; without spaces."""
;
;; # Find the best match for the i first characters, assuming cost has
;; # been built for the i-1 first characters.
;; # Returns a pair (match_cost, match_length).
;; def best_match(i):
;; candidates = enumerate(reversed(cost[max(0, i-maxword):i]))
;; return min((c + wordcost.get(s[i-k-1:i], 9e999), k+1) for k,c in candidates)
(defn- best-match
[i s cost]
(let [candidates (reverse (subvec cost (max 0 (- i max-word)) i))]
(apply min-key first (map-indexed (fn [k c] [(+ c (get-word-cost (subs s (- i k 1) i) 9e9999)) (inc k)]) candidates))))
;; # Build the cost array.
;; cost = [0]
;; for i in range(1,len(s)+1):
;; c,k = best_match(i)
;; cost.append(c)
(defn- build-cost-array
[s]
(loop [i 1
cost [0]]
(if-not (< i (inc (count s)))
cost
(recur (inc i)
(conj cost (first (best-match i s cost)))))))
;; # Backtrack to recover the minimal-cost string.
;; out = []
;; i = len(s)
;; while i>0:
;; c,k = best_match(i)
;; assert c == cost[i]
;; out.append(s[i-k:i])
;; i -= k
;;
;; return " ".join(reversed(out))
(defn infer-spaces
"Splits a string with no spaces into words using magic" ; what a great explanation. TODO - make this code readable
[input]
(let [s (str/lower-case input)
cost (build-cost-array s)]
(loop [i (float (count s))
out []]
(if-not (pos? i)
(reverse out)
(let [[c k] (best-match i s cost)]
(recur (- i k)
(conj out (subs s (- i k) i))))))))
src/metabase/util/stats.clj
+ 1
− 0
View file @ c3f97bbb
......@@ -114,6 +114,7 @@
:check_for_updates (public-settings/check-for-updates)
:site_name (not= (public-settings/site-name) "Metabase")
:report_timezone (driver/report-timezone)
; We deprecated advanced humanization but have this here anyways
:friendly_names (= (humanization/humanization-strategy) "advanced")
:email_configured (email/email-configured?)
:slack_configured (slack/slack-configured?)
......
test/metabase/models/humanization_test.clj
+ 13
− 84
View file @ c3f97bbb
......@@ -6,60 +6,6 @@
[toucan.db :as db]
[toucan.util.test :as tt]))
(deftest advanced-humanization-test
(doseq [[input expected] {nil nil
"" nil
"_" ""
"-" ""
"_id" "ID"
"uid" "UID"
"uuid" "UUID"
"guid" "GUID"
"ip" "IP"
"url" "URL"
"_agent_invite_migration" "Agent Invite Migration"
"-agent-invite-migration" "Agent Invite Migration"
"fooBar" "Foo Bar"
"foo-bar" "Foo Bar"
"foo_bar" "Foo Bar"
"foo bar" "Foo Bar"
"dashboardcardsubscription" "Dashboard Card Subscription"
"foo_id" "Foo ID"
"receiver_id" "Receiver ID"
"inbox" "Inbox"
"acquirer" "Acquirer"
"auth_authenticator" "Auth Authenticator"
"authprovider" "Auth Provider"
"usersocialauth" "User Social Auth"
"allalter" "All Alter"
"alterall" "Alter All"
"andanyor" "And Any Or"
"ascdesc" "Ascdesc"
"decimaldefaultdelete" "Decimal Default Delete"
"elseendexist" "Else End Exist"
"forfrom" "For From"
"gotograntgroup" "Goto Grant Group"
"ifininsertis" "If In Insert Is"
"notnull" "Not Null"
"ofonorder" "Of On Order"
"rangeselect" "Range Select"
"tablethentotype" "Table Then To Type"
"unionuniqueupdate" "Union Unique Update"
"valueswherewith" "Values Where With"
"changelog" "Changelog"
"dataflow" "Dataflow"
"bestseller" "Bestseller"
"uniques" "Uniques"
"cpi" "Cpi"
"ctr" "Ctr"
"paid_to_sparkify" "Paid To Sparkify"
"paidtosparkify" "Paid To Sparkify"
"assassinate" "Assassinate"
"analbumcover" "An Album Cover"}]
(testing (pr-str (list 'name->human-readable-name :advanced input))
(is (= expected
(humanization/name->human-readable-name :advanced input))))))
(deftest simple-humanization-test
(doseq [[input expected] {nil nil
"" nil
......@@ -119,25 +65,16 @@
(humanization/name->human-readable-name :none input))))))
(deftest db-inspired-test
(doseq [[input strategy->expected] {"sumsubtotal" {:advanced "Sum Subtotal"}
"sum(subtotal)" {:advanced "Sum(subtotal)"
:simple "Sum(subtotal)"
(doseq [[input strategy->expected] {"sum(subtotal)" {:simple "Sum(subtotal)"
:none "sum(subtotal)"}
"created_at::date" {:advanced "Created At::date"
:simple "Created At::date"
"created_at::date" {:simple "Created At::date"
:none "created_at::date"}
"datecreated" {:advanced "Date Created"
:simple "Datecreated"
"datecreated" {:simple "Datecreated"
:none "datecreated"}
"createdat" {:advanced "Created At"
:simple "Createdat"
"createdat" {:simple "Createdat"
:none "createdat"}
"createat" {:advanced "Create At"}
"updatedat" {:advanced "Updated At"
:simple "Updatedat"
"updatedat" {:simple "Updatedat"
:none "updatedat"}
"updateat" {:advanced "Update At"}
"castcreatedatasdate" {:advanced "Cast Created At As Date"}
"cast(createdatasdate)" {:simple "Cast(createdatasdate)"
:none "cast(createdatasdate)"}}
[strategy expected] strategy->expected]
......@@ -151,12 +88,10 @@
(db/select-one-field :display_name Table, :id table-id))))
(deftest humanized-display-name-test
(testing "check that we get the expected :display_name with advanced humanization *enabled*"
(doseq [[input strategy->expected] {"toucansare_cool" {"advanced" "Toucans Are Cool"
"simple" "Toucansare Cool"
(testing "check that we get the expected :display_name with humanization *enabled*"
(doseq [[input strategy->expected] {"toucansare_cool" {"simple" "Toucansare Cool"
"none" "toucansare_cool"}
"fussybird_sightings" {"advanced" "Fussy Bird Sightings"
"simple" "Fussybird Sightings"
"fussybird_sightings" {"simple" "Fussybird Sightings"
"none" "fussybird_sightings"}}
[strategy expected] strategy->expected]
(testing (pr-str (list 'get-humanized-display-name input strategy))
......@@ -165,15 +100,13 @@
(deftest rehumanize-test
(testing "check that existing tables have their :display_names updated appropriately when strategy is changed"
(doseq [[actual-name expected] {"toucansare_cool" {:initial "Toucans Are Cool"
(doseq [[actual-name expected] {"toucansare_cool" {:initial "Toucansare Cool"
:simple "Toucansare Cool"
:advanced "Toucans Are Cool"
:none "toucansare_cool"}
"fussybird_sightings" {:initial "Fussy Bird Sightings"
"fussybird_sightings" {:initial "Fussybird Sightings"
:simple "Fussybird Sightings"
:advanced "Fussy Bird Sightings"
:none "fussybird_sightings"}}]
(tu/with-temporary-setting-values [humanization-strategy "advanced"]
(tu/with-temporary-setting-values [humanization-strategy "simple"]
(tt/with-temp Table [{table-id :id} {:name actual-name}]
(letfn [(display-name [] (db/select-one-field :display_name Table, :id table-id))]
(testing "initial display name"
......@@ -183,10 +116,6 @@
(humanization/humanization-strategy "simple")
(is (= (:simple expected)
(display-name))))
(testing "switch to :advanced"
(humanization/humanization-strategy "advanced")
(is (= (:advanced expected)
(display-name))))
(testing "switch to :none"
(humanization/humanization-strategy "none")
(is (= (:none expected)
......@@ -194,10 +123,10 @@
(deftest do-not-overwrite-custom-names-test
(testing "check that if we give a field a custom display_name that changing strategy doesn't overwrite it"
(doseq [initial-strategy ["advanced" "simple" "none"]]
(doseq [initial-strategy ["simple" "none"]]
(tu/with-temporary-setting-values [humanization-strategy initial-strategy]
(tt/with-temp Table [{table-id :id} {:name "toucansare_cool", :display_name "My Favorite Table"}]
(doseq [new-strategy ["advanced" "simple" "none"]]
(doseq [new-strategy ["simple" "none"]]
(testing (format "switch from %s -> %s" initial-strategy new-strategy)
(humanization/humanization-strategy new-strategy)
(is (= "My Favorite Table"
......
test/metabase/util/infer_spaces_test.clj deleted 100644 → 0
+ 0
− 15
View file @ 4347464b
(ns metabase.util.infer-spaces-test
(:require [clojure.test :refer :all]
[metabase.util.infer-spaces :as infer-spaces]))
(deftest infer-spaces-test
(doseq [[input expected] {"user" ["user"]
"users" ["users"]
"orders" ["orders"]
"products" ["products"]
"events" ["events"]
"checkins" ["checkins"]
"dashboardcardsubscription" ["dashboard" "card" "subscription"]}]
(testing (pr-str (list 'infer-spaces input))
(is (= expected
(infer-spaces/infer-spaces input))))))
test/metabase/util/stats_test.clj
+ 1
− 1
View file @ c3f97bbb
......@@ -72,7 +72,7 @@
(doseq [[k expected] {:running_on :unknown
:check_for_updates true
:site_name true
:friendly_names true
:friendly_names false
:email_configured false
:slack_configured false
:sso_configured false
......
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