elm-community / list-extra

Convenience functions for working with List.
http://package.elm-lang.org/packages/elm-community/list-extra/latest
MIT License
135 stars 59 forks source link

Improve groupsOfWithStep & greedyGroupsOfWithStep Documentation #53

Closed prikhi closed 7 years ago

prikhi commented 7 years ago

The docs for groupsOfWithStep say:

Split list into groups of size given by the first argument. After each group, drop a number of elements given by the second argument before starting the next group.

With this example & test:

groupsOfWithStep 2 1 (range 1 4) == [[1,2],[2,3],[3,4]]

But shouldn't it be something like this(groupsOfWithStep 2 1 (range 1 6)):

[1, 2, 3, 4, 5, 6]
=> [ [1, 2] ]
=> [ [1, 2], [4, 5] ]

It should take 2, then drop 1, then take 2, right? Unless I'm misunderstanding the documentation.

Edit: Same with greedyGroupsOfWithStep?

greedyGroupsOfWithStep 3 2 (range 1 6) == [[1,2,3],[3,4,5],[5,6]]

Should be [ [1, 2, 3], [6] ]?

pzp1997 commented 7 years ago

I looked at the author's commit message, and they referenced the partition function from Clojure. Based on the documentation for that function, it looks like the intended meaning is to drop step number of elements counting starting from the previous partition before continuing. If step is the same as the partition size, each element will be contained in exactly one partition (except for perhaps towards the end depending on greediness). If it is less than the partition size, there will be overlap in the partitions. If it is greater than the partition size, some elements will be skipped. I can submit a PR to fix the documentation in the morning.

prikhi commented 7 years ago

@pzp1997 Thanks for checking it out, I updated the issue title to reflect what you found.