Skip to content
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

Add a samplePercent pipeline to randomly sample a stream #8058

Merged
merged 4 commits into from
Jun 1, 2023

Conversation

gnp
Copy link
Contributor

@gnp gnp commented Apr 18, 2023

No description provided.

Copy link
Contributor

@adamgfraser adamgfraser left a comment

Choose a reason for hiding this comment

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

Maybe just sample?

@gnp
Copy link
Contributor Author

gnp commented Apr 19, 2023

Maybe just sample?

I chose samplePercent because there are other ways of sampling. For example, I'm also planning to do sampleN where you specify a number of elements you want from an arbitrary (finite) length stream. That one probably will be implemented on ZSink though. Then, sample1 is a slight optimization that would have element instead of chunk success type. These could reasonably called chooseN and choose1 instead.

That said, I would not object to calling this one just sample if that is important to you, since that is the main and obvious thing. If ever there are variants, it could be those that have a qualifier suffix.

@gnp
Copy link
Contributor Author

gnp commented Apr 19, 2023

I added one additional test and changed the name.

@adamgfraser
Copy link
Contributor

That gets a bit to my other concern which is that sampling can be its own entire domain so potentially this belongs in a library that is based on ZIO Stream that focuses on that.

@gnp
Copy link
Contributor Author

gnp commented Apr 19, 2023

I don't think I'm in a good position to design a larger stats / random based stream utilities module. I can see the need for ZPipeline.sample, ZSink.chooseN and ZSink.choose1 because I needed choose1 for some testing I was doing and the others are obvious.

@gnp
Copy link
Contributor Author

gnp commented Jun 1, 2023

Still want this contribution @adamgfraser?

@adamgfraser adamgfraser merged commit 4fc8f6d into zio:series/2.x Jun 1, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants