Skip to content

Add Stream::unzip #232

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

Closed
wants to merge 1 commit into from
Closed

Add Stream::unzip #232

wants to merge 1 commit into from

Conversation

tirr-c
Copy link
Contributor

@tirr-c tirr-c commented Sep 22, 2019

Iterator::unzip collects elements in Default + Extend containers, but here I used FromStream on bound channels with no capacity.

@yoshuawuyts
Copy link
Contributor

Ref #129.

@tirr-c Thanks so much for starting this! Could you say more about why you chose to change the trait bounds? We've just merged #211, which makes it possible to use in the bounds.

Heh also def seems like this would pair up nicely with the channel design in #212! (though we shouldn't block ourselves on it I think).

@tirr-c
Copy link
Contributor Author

tirr-c commented Sep 23, 2019

@yoshuawuyts I first thought that Default + Extend bounds meant it'll make a new container and extend from some iterator (or stream), so I used FromStream instead.

I've just checked out the implementation of Iterator::unzip. Surprisingly it was using Extend as an abstraction of element addition! So we can even use Default + std::iter::Extend for the container bounds.

@yoshuawuyts
Copy link
Contributor

So we can even use Default + std::iter::Extend for the container bounds.

Not necessarily -- if Extend is implemented for some kind of database handle then std::iter::Extend would not be suitable. I think if we want people to be able to experiment with building networked streaming patterns we keep it as Default + Stream::Extend.

Refs

@tirr-c
Copy link
Contributor Author

tirr-c commented Sep 23, 2019

If we're not given any existing database handle, we need to build a new handle from its default impl; that isn't what we want, I think. How about unzip returning two streams?

@yoshuawuyts yoshuawuyts added the enhancement New feature or request label Sep 28, 2019
@k-nasa k-nasa mentioned this pull request Nov 16, 2019
@tirr-c
Copy link
Contributor Author

tirr-c commented Nov 16, 2019

This PR is superseded by #543.

@tirr-c tirr-c closed this Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants