redplanetlabs / specter

Clojure(Script)'s missing piece
Apache License 2.0
2.52k stars 105 forks source link

Question about multi-transform #246

Closed DjebbZ closed 6 years ago

DjebbZ commented 6 years ago

Hello,

I want to transform a vector of maps in several places :

The theoritical code looks like this :

(sp/multi-transform
  [sp/ALL (sp/multi-path [:values sp/MAP-KEYS #{:a} (sp/terminal-val sp/NONE)] ;; 1. :values submap entries with some keys
                         [#(< (count (:values %)) 2) (sp/terminal-val sp/NONE)] ;; 2. top-level maps with :values submap too small
                         (sp/terminal transform-a-map))] ;; 3. transform the rest
  vector-of-maps)

However this code fails with java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to clojure.lang.Associative

After putting some println I found that terminaling with sp/NONE indeed puts sp/NONE for the next transform to operate. Which means my transform-a-map function fails when it receives sp/NONE.

I managed to make it work by putting the last step at the first position (3-1-2)) but it's suboptimal since it will transform all maps instead of only those relevant. In fact I quick-bench with criterium the version that works and another version that just thread-last into setval 1 setval 2 transform 3 and the thread-last version is slightly faster.

My question is : is it possible to write an efficient and "logical" (keeping the order 1 2 3) multi-transform ?

nathanmarz commented 6 years ago

You can write it like this:

(sp/multi-transform
  [sp/ALL (sp/multi-path [:values (sp/map-key :a) (sp/terminal-val sp/NONE)]
                         [#(< (count (:values %)) 2) (sp/terminal-val sp/NONE)]
                         [#(not (identical? sp/NONE %)) (sp/terminal transform-a-map))]]
  vector-of-maps)

Also note the use of map-key which is much more efficient than what you were doing.

DjebbZ commented 6 years ago

Thank you, haven't thought about identical?.

To be honest my #{:a} was a simplification, I'm trying to remove all map entries with keys not in a set of keywords. So the navigator of step 1 is like [sp/ALL :values sp/MAP-KEYS #(not (valid-keys %))]. Haven't found a way to express this with Specter.

jiacai2050 commented 6 years ago

Similar question here.

AFAIK, multi-path works well only when path are static. For example:

(let [m {:k1 1
         :k2 2
         :k3 3}]
  (multi-transform [(multi-path [:k1 (terminal inc)]
                                [:k2 (terminal #(* % %))]
                                [:k3 (terminal dec)])]
          m))
;; => {:k1 2, :k2 4, :k3 2}

This example is both elegant and effective, but when I want to multi-transform on a dynamic path, I have no idea what to do with specter, for example:

(let [m {:app1 {:k1 1}
         :app2 {:k2 2}
         :app3 {:k3 3}}
      where1 #{[:app1 :k1] [:app2 :k2]}
      where2 #{[:app3 :k3]}]
  (multi-transform [(multi-path [where1 do-something]
                                [where2 do-sth-else])]
          m))

@nathanmarz @DjebbZ any suggestions ? related: https://github.com/nathanmarz/specter/issues/248

nathanmarz commented 6 years ago

@jiacai2050 Parameterized navigators are just functions, so you can use/compose them just like anything else:

(defn to-keypath [keys] (apply keypath keys))
(defn multi-keypath [keys-set] (transduce (map to-keypath) multi-path keys-set1))

(defn foo [m keys-set1 keys-set2]
  (let [p1 (multi-keypath keys-set1)
        p2 (multi-keypath keys-set2)]
    (multi-transform
      (multi-path [p1 do-something] [p2 do-sth-else])
      m
      )))
jiacai2050 commented 6 years ago

@nathanmarz That's awesome. I haven't realized navigators can be composed just like functions, this give so much power, I can't imagine how I spend my life writing Clojure without specter before.

Yesterday I watched your talk which teaches me a lot, and I suggest user->bank demo should be added in README.md, or at least navigators composition should be introduced somewhere in README.

nathanmarz commented 6 years ago

@jiacai2050 That's a good suggestion to add to #56

jiacai2050 commented 6 years ago

👍 I will check this issue later. It seems that specter-wiki repo is the preferred docs, but there's no mention in README, intended or something else ?

nathanmarz commented 6 years ago

All the pages on the wiki are linked from the README

jiacai2050 commented 6 years ago

README contains links to https://github.com/nathanmarz/specter/wiki not https://github.com/nathanmarz/specter-wiki , is there any connection between them?

nathanmarz commented 6 years ago

specter-wiki is the source for specter/wiki. The repository exists so people can make pull requests to it.

jiacai2050 commented 6 years ago

Gotcha, does this mean that after merge a PR, you will update the wiki page manually? If yes, it seems kind of awkward, why not just use the repo for docs?

nathanmarz commented 6 years ago

No, it's not manual. It's just a git push.

jiacai2050 commented 6 years ago

That makes sense. Thanks.