medley
medley copied to clipboard
weavejester
Retain metadata for polyadic assoc-some
On a related note, reduce-map could benefit from this too:
https://github.com/tomdl89/medley/commit/8fd5920491cf15deb55d30baddab69bbf908ee64
I'm not convinced https://github.com/weavejester/medley/issues/13 reached the right conclusion btw. Comparing to map filter remove etc doesn't seem to make sense, as those functions produce a lazy-seq from a potentially different coll, whereas map-vals filter-vals remove-vals etc always take a map and return a map. Indeed, update-vals in latest clojure versions (which is doing the same thing as map-vals) does retain metadata.
Thanks for the commit, and apologies for the delay in reviewing it. Can you add a simple test just to prevent any regressions in future.
With regard to #13, I was originally basing my decision on mapv, which takes a vector and returns a vector, but discards the metadata. It's interesting that with update-vals they've decided to go the other way.
I'm willing to keep metadata for maps if that's the precedent set by clojure.core. I'm just concerned about keeping behavior consistent.
I think the reasoning is that functions that modify an object retain metadata, and sequence functions don't. update-vals follows after assoc and update by retaining metadata, whereas mapv follows after map which does not. I would expect an assoc-like function to retain metadata.
Then perhaps the rule is that functions with the collection as the first argument retain metadata, and otherwise it doesn't. Hence update-vals retaining metadata, and mapv not doing so.
Looks like this PR is still waiting on a test. Would you be able to write one @tomdl89, or do you not have time? If needs be, I can write one.
Thanks! Could you squash the tests commit into the original? Then I'll merge it into master.