Skip to content

Conversation

igas
Copy link
Contributor

@igas igas commented Feb 1, 2015

While reviewing #3016 I found a bug in chunk_by:

stream = Stream.chunk_by([1, 2, 2, 3, 4, 4, 6, 7, 7], &(rem(&1, 2) == 1))
stream |> Enum.take(2)
# => [[1], [2, 2], [3]]

Copy link
Member

Choose a reason for hiding this comment

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

Please avoid changing style of other assertions as it makes it hard to see what is being changed as part of this change. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that, I just went through each test and add take assertion where it missed, to prevent such things in future. Below I added line 85.

@josevalim
Copy link
Member

Good catch!

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately this fix is not correct. We cannot access the accumulator state because the take value can be nested inside other states or it could just mean whatever. :)

@josevalim josevalim closed this in e3399be Feb 1, 2015
@josevalim
Copy link
Member

Thanks for finding this out! I have pushed a fix to Enum.take/2 as it was continue to emit items when counter reached 0!

@igas
Copy link
Contributor Author

igas commented Feb 1, 2015

👍

josevalim pushed a commit that referenced this pull request Feb 2, 2015
Signed-off-by: José Valim <jose.valim@plataformatec.com.br>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants