Skip to content

Conversation

@jgmchan
Copy link
Contributor

@jgmchan jgmchan commented Jan 18, 2018

Before v1.6.0 the following code worked:

 iex(1)> Stream.chunk([0, 1, 2, 3], 2) |> Stream.zip() |> Enum.to_list()
[{0, 2}, {1, 3}]

However, we now get an error in versions >=1.6.0:

iex(1)> Stream.chunk([0, 1, 2, 3], 2) |> Stream.zip() |> Enum.to_list()

** (FunctionClauseError) no function clause matching in Stream.zip/1

    The following arguments were given to Stream.zip/1:

        # 1
        #Stream<[
          enum: [0, 1, 2, 3],
          funs: [#Function<3.91433161/1 in Stream.chunk_while/4>]
        ]>

    (elixir) lib/stream.ex:1110: Stream.zip/1

This happens with Enum.zip/1 as well.

It looks like the behaviour of the zip/1 function was changed in #6322.

If this is not the intended behaviour then this PR will allow the pre-v1.6.0 code to work again. Otherwise, I'm happy to close this PR and update the documentation to state that zip/1 can only work with lists of enumerables and not streams.

This was working previously before Elixir v1.6.0

def zip(enumerables) when is_list(enumerables), do: do_zip(enumerables)

defp do_zip(enumerables) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be much cleaner if we can have a guard that checks that enumerable is a list or a %Stream{} struct but I'm not sure how to do that.


def zip(enumerables) when is_list(enumerables), do: zip_fun(enumerables)

defp zip_fun(enumerables) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a a pretty bad name but I can't think of a better one 😄 . I thought about calling it _zip but I think it seems to clash with the convention of unimported functions. Any advice on this would be great!

@jgmchan
Copy link
Contributor Author

jgmchan commented Jan 18, 2018

Hmm, failed tests. Will look into it.

The `Enum` module is part of the bootstrap of Elixir so it doesn't
understand the `%Stream{}` pattern matching just yet.
def zip([]), do: []

def zip(enumerables) when is_list(enumerables) do
def zip(%{__struct__: Stream} = enumerables), do: do_zip(enumerables)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used pattern matching for the __struct__ key because clean compilation fails if I use this:

def zip(%Stream{} = enumerables), do: do_zip(enumerables)

I'm wondering if we need to check the enumerable type here if Stream.zip/1 already does it.

@josevalim josevalim closed this in 69bee9a Jan 19, 2018
josevalim pushed a commit that referenced this pull request Jan 19, 2018
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.

1 participant