-
Notifications
You must be signed in to change notification settings - Fork 69
Add window function #98
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.
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
Conversation
weavejester
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. My initial impression is that this is reasonable, but let me give it a some thought first.
src/medley/core.cljc
Outdated
| when no collection is provided. For a sliding window containing each element | ||
| and n-1 _following_ elements, use `clojure.core/partition` with a `step` size | ||
| of 1." | ||
| {:added "1.10.0"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 1.9.0 I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sorry I assumed 1.9 had been cut. Will correct.
|
Sorry for the delay @weavejester - I've corrected the added metadata, and assume the benchmarking is satisfactory? Cheers |
|
I probably won't be able to look at this in depth for a couple of weeks due to conference organizing. I wanted to let you know that this is on my radar though. |
|
No worries. reClojure by any chance? If so, I'll see you there! |
|
That's the one! |
|
I think all looks good. Can you squish down the commits? |
|
All squished. lmk if you need anything else. Cheers |
|
Hi @weavejester can you cut a release when you get a chance please? |
Released 1.9.0. Sorry for the delay. |
|
No worries, thank you! |
I've had the lazy-only version of this function in quite a few codebases over the years, and recently took the time to write the transducer version. The cljs support is particularly annoying to write because cljs's
array-listtype doesn't implement aremovemethod.This is like a simple alternative to https://github.com/cgrand/xforms/ 's
windowtransducer, which needsfandinvffunctions to construct the windows. Plus this has a standard lazy-arity.An example use case: