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

Not sure on the difference between std::chunk_by and itertools::chunk_by #1006

Open
arifd opened this issue Dec 17, 2024 · 3 comments
Open

Comments

@arifd
Copy link

arifd commented Dec 17, 2024

Hello!

std has chunk_by and so does itertools

It seems to me, that the only difference is that std does not return the value of the functor, But I'm not sure, and don't want to introduce a subtle error by switching over if i don't need the key.

Could we include a small comment in the documentation to help users choose between itertools' and std's implementation?

@phimuemue
Copy link
Member

phimuemue commented Dec 17, 2024

Thanks for bringing this to our attention.

I think your intuition that they yield the same chunks is correct.

slice::chunk_by works on slices and when you iterate over it, you'll get slices. Its predicate has the form FnMut(&T, &T)->bool, i.e. you tell it if two consecutive elements belong to the same chunk.

Itertools::chunk_by works on iterators. This is a bit more general, but needs more work compared to the pure slice version. iirc, it's a bit more cumbersome to use, too. Its functor has the form FnMut(Item)->K and it requires K: PartialEq, so that this functor extracts a "key" from the item, and uses K's equality to determine if they belong to the same chunk.

I think we should adopt what std does on slices here: In particular, our closure should be a predicate (seems strictly more general that what we currently have) and the documentation should be in line with std's. @jswrenn Thoughts?

@arifd
Copy link
Author

arifd commented Dec 20, 2024

Ah right. Yes, thanks for the explanation!

Yes, bool seems to me, more intuitive and perhaps more useful too.

I'm happy to close this issue since you've covered my concern, but I'll leave it open for someone else to close, since an interesting question has been posed.

@jswrenn
Copy link
Member

jswrenn commented Dec 26, 2024

I think we should adopt what std does on slices here: In particular, our closure should be a predicate (seems strictly more general that what we currently have) and the documentation should be in line with std's. @jswrenn Thoughts?

Yes, agreed.

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

No branches or pull requests

3 participants