Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improves (map) #193

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

improves (map) #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
437995e
improves (map)
agzam Aug 12, 2024
4d2512e
removes concat's dup
agzam Aug 22, 2024
9f9cf8b
don't unpack, apply does it already
agzam Aug 22, 2024
09ef995
adds basic (map) tests
agzam Aug 22, 2024
c014146
export apply
agzam Aug 22, 2024
2a753cd
eq? -> seq-eq?
agzam Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 40 additions & 20 deletions lib/functional.fnl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@
(set ct (+ ct 1))))
ct)

(fn apply [f ...]
agzam marked this conversation as resolved.
Show resolved Hide resolved
(let [args [...]]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have to assign the name here or can you just return (f (table.unpack ...))?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to 'double nest' it. E.g. if I call (apply add [1 2]) in apply args is { { 1, 2} }

Copy link
Owner Author

@agzam agzam Aug 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think (f (table.unpack ...)) works.

if I call (apply add [1 2])

You're giving it a single argument. (apply) usually works with multiple, in your case you'd call it (apply add 1 2)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of apply, then? Couldn't you just call (add 1 2)?. As I understand it (from clojure), (apply add [1 2])) should be equivalent to (add 1 2)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you given this one more thought?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I figure we should probably work in the same way as clojure

Copy link
Owner Author

@agzam agzam Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put a pin on this one. It definitely requires some more thought. Even fennel-cljlib implementation is not so trivial.
We need to handle cases like:

user> (apply print [1 2 3])
1 2 3;; => nil
user> (apply print 1 [2 3])
1 2 3;; => nil
user> (apply print [1 2] [3 4])
[1 2] 3 4;; => nil

Why doesn't Clojure unpack both vectors in the last case I have no idea, but that's how it seems to work.
Seven peduncles, I had no idea how Clojure's (apply) is unpredictable, lol:

user> (apply print [:one :two :three])
:one :two :three;; => nil

user> (apply print [:one :two :three] [:four])
[:one :two :three] :four;; => nil

user> (apply print :one :two :three)
java.lang.IllegalArgumentException: Don't know how to create ISeq from: clojure.lang.Keyword 

(apply print [:one :two :three] :four)
java.lang.IllegalArgumentException: Don't know how to create ISeq from: clojure.lang.Keyword

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh that's strange. I don't think I have tried to use apply with more than 2 args (a function and a sequence).

I am ok with figuring this out later. We can remove this and merge the rest?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm... apply is used though. in (map) particularly. I'd say let's merge it as is and then maybe improve it later? I think for general use cases it works. Or do you prefer to get it right instead and not to increase the tech-debt budget?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that clojure basically only unpack the last argument, which must be a sequence. This implementation collects all args into a table and then unpacks that, which is also incorrect.

map only uses apply with two args, so what I think might be best would be to write this apply for 2 args [fn seq] and unpack that seq, we can later add support for extra intermediate non-splices args between the function and the last sequence.

(f (table.unpack args))))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Reduce Primitives
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand All @@ -145,6 +149,11 @@
k v (seq tbl)]
(f acc v k)))

(fn concat [...]
agzam marked this conversation as resolved.
Show resolved Hide resolved
(reduce (fn [cat tbl]
(each [_ v (ipairs tbl)]
(table.insert cat v))
cat) [] [...]))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Reducers
Expand All @@ -162,14 +171,33 @@
tbl
paths))

(fn map
[f tbl]
(reduce
(fn [new-tbl v k]
(table.insert new-tbl (f v k))
new-tbl)
[]
tbl))
(fn zip [...]
"Groups corresponding elements from multiple lists into a new list, truncating at the length of the smallest list."
(let [tbls [...]
result []]
(if (= 1 (length tbls))
(table.insert result (. tbls 1))
(for [idx 1 (length (. tbls 1))]
(let [inner []]
(each [_ tbl (ipairs tbls) &until (not (. tbl idx))]
(table.insert inner (. tbl idx)))
(table.insert result inner))))
result))

(fn map [f ...]
(let [args [...]
tbls (zip (table.unpack args))]
(if (= 1 (count args))
(icollect [_ v (pairs (first args))]
(apply f v))
(accumulate [acc []
agzam marked this conversation as resolved.
Show resolved Hide resolved
_ t (ipairs tbls)]
(concat acc [(apply f t)])))))

(fn map-kv [f coll]
"Maps through an associative table, passing each k/v pair to f"
(icollect [k v (pairs coll)]
(f k v)))

(fn merge
[...]
Expand All @@ -192,16 +220,6 @@
[]
tbl))

(fn concat
[...]
(reduce
(fn [cat tbl]
(each [_ v (ipairs tbl)]
(table.insert cat v))
cat)
[]
[...]))

(fn some
[f tbl]
(let [filtered (filter f tbl)]
Expand Down Expand Up @@ -237,7 +255,8 @@
;; Exports
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

{: butlast
{: apply
: butlast
: call-when
: compose
: concat
Expand All @@ -257,12 +276,13 @@
: last
: logf
: map
: map-kv
: merge
: noop
: reduce
: seq
: seq?
: some
: slice
: some
: split
: tap}
26 changes: 25 additions & 1 deletion test/functional-test.fnl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,28 @@
(fn []
(is.eq? (f.reduce #(.. $1 $2) "" [5 4 3 2 1]) "54321" "reduce did not concat list into string")
(is.eq? (f.reduce #(if (> $1 $3) $1 $3) 0 [1 3 5 2 0]) 5 "reduce did not find max")))
))

(it "(map) traverses a table"
(fn []
(is.seq-eq?
(f.map (fn [x] x) [:a :b :c])
[:a :b :c]
"same table")))

(it "(map) traverses a table with a transform"
(fn []
(is.seq-eq?
(f.map (fn [x] (string.upper x)) [:a :b :c])
[:A :B :C]
"capitalized")))

(it "(map) traverses multiple tables"
(fn []
(is.eq?
(fennel.view
(f.map
(fn [a b]
(f.concat b a)) [:a :b :c] [1 2 3]))
(fennel.view
[[:a 1] [:b 2] [:c 3]])
"data from both tables appear")))))
10 changes: 7 additions & 3 deletions windows.fnl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
: count
: concat
: contains?
: apply
Grazfather marked this conversation as resolved.
Show resolved Hide resolved
: map
: map-kv
: for-each
: split} (require :lib.functional))
: split
: logf
} (require :lib.functional))
(local {:global-filter global-filter} (require :lib.utils))
(local {:atom atom
:deref deref
Expand Down Expand Up @@ -436,7 +440,7 @@
(reset! screen-number-canvases []))

(fn monitor-item
[screen i]
[i screen]
"
Creates a menu item to move the frontMost window to the specified screen index
Takes a hs.screen instance and an index integer
Expand Down Expand Up @@ -471,7 +475,7 @@
Returns mutated modal menu table-map
"
(->> screens
(map monitor-item)
(map-kv monitor-item)
(concat menu.items)
(tset menu :items))
menu)
Expand Down