Skip to content

Improved zip API #18769

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Improved zip API #18769

wants to merge 4 commits into from

Conversation

dennisvennink
Copy link
Contributor

@dennisvennink dennisvennink commented Aug 16, 2018 •
edited
Loading

This PR contains the implementation, documentation, tests and benchmarks for the Improved zip API proposal.

@jrose-apple jrose-apple added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Aug 16, 2018
@dennisvennink dennisvennink changed the title Add zipLongest(_:_:) Add zipLongest(_:_:) (WIP) Aug 17, 2018
airspeedswift
airspeedswift reviewed Aug 18, 2018
View reviewed changes
@airspeedswift
Copy link
Member

Can you add a benchmark as well? Thanks!

@dennisvennink dennisvennink changed the title Add zipLongest(_:_:) (WIP) Expand zip API (WIP) Oct 29, 2018
@dennisvennink dennisvennink changed the title Expand zip API (WIP) Improved zip API Nov 10, 2018
@dennisvennink dennisvennink force-pushed the add-ziplongest branch 6 times, most recently from fc36f46 to 30fef1f Compare December 13, 2018 12:45
@lantua
Copy link

lantua commented Dec 23, 2018

In underestimatdCount implementation, do we really need check if result >= 0?
Many implementations that uses it should already gracefully handle (or simply ignore) this pathological case.

@dennisvennink
Copy link
Contributor Author

dennisvennink commented Dec 23, 2018 •
edited
Loading

In underestimatdCount implementation, do we really need check if result >= 0?

While the default implementation returns 0, it is not guaranteed. And because we're using min(_:_:) it's probably a good idea to be defensive.

Edit: The clamping check is removed and has already been merged.

@dennisvennink
Copy link
Contributor Author

Can you add a benchmark as well? Thanks!

@airspeedswift I've added some Iterator-related benchmarks. Do you think we should add others as well? And if so, which?

@shahmishal
Copy link
Member

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

  • How to change the base branch: (Link)
  • More detail about the branch update: (Link)

@shahmishal shahmishal closed this Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@airspeedswift airspeedswift airspeedswift left review comments

Assignees
No one assigned
Labels
swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dennisvennink @airspeedswift @lantua @shahmishal @jrose-apple