rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.72k stars 309 forks source link

Document order guarantees for combinations #822

Closed ryanavella closed 9 months ago

ryanavella commented 10 months ago

Fixes #819 by documenting the order guarantees for Itertools adapters.

Philippe-Cholet commented 10 months ago

CI failed because rustfmt does not accept trailing spaces.

The question is if we want this guarantee to be library wide or limited to some adaptors like Combinations. I'll let @jswrenn decide on that.

ryanavella commented 10 months ago

The question is if we want this guarantee to be library wide or limited to some adaptors like Combinations. I'll let @jswrenn decide on that.

Ah yes, I hope I didn't misunderstand the conclusion from the other issue. I originally wanted a guarantee for Combination-like adapters, but I interpreted your comments as saying we don't intend to break iteration order for any adapters.

I could add a blanket "unless otherwise noted" clause, if there are specific adapters we'd like to leave open to further optimizations?

jswrenn commented 10 months ago

Sorry for the misunderstanding. I don't think we can guarantee order for all adaptors, but I am okay guaranteeing it for the combinations adaptors.

ryanavella commented 10 months ago

Thank you for the clarification, I've updated this PR so that the guarantee is only shown under Itertools::combinations and Itertools::tuple_combinations. Let me know if all looks good to you.

Philippe-Cholet commented 9 months ago

@jswrenn What do you think of his formulation?

Philippe-Cholet commented 9 months ago

@ryanavella If you could commit jswrenn' suggestions and squash into a single commit, I would gladly merge this.

EDIT: Done.

Philippe-Cholet commented 9 months ago

@jswrenn My approval is not enough to get this merged.