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

Fix Iter.take to handle infinite iterator #2212

Merged
merged 1 commit into from
Sep 6, 2017
Merged

Fix Iter.take to handle infinite iterator #2212

merged 1 commit into from
Sep 6, 2017

Conversation

Theodus
Copy link
Contributor

@Theodus Theodus commented Sep 3, 2017

Previously, the take method on an infinite stream, such as Rand,
would cause infinite recursion.

@Theodus Theodus added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Sep 3, 2017
@SeanTAllen
Copy link
Member

This feels like something that it would be good to have a test for.

Is it something we could do with a test that has a timeout? If it doesn't complete within X, fail the test?

@Theodus
Copy link
Contributor Author

Theodus commented Sep 3, 2017

@SeanTAllen the test that I added segfaults prior to this change, though a timeout would be better.

Previously, the `take` method on an infinite stream, such as `Rand`,
would cause infinite recursion.
@Theodus
Copy link
Contributor Author

Theodus commented Sep 3, 2017

@SeanTAllen the test now has a timeout

SeanTAllen referenced this pull request Sep 6, 2017
Previously, the `take` method on an infinite stream, such as `Rand`,
would cause infinite recursion.
@SeanTAllen SeanTAllen merged commit 0595011 into master Sep 6, 2017
@SeanTAllen SeanTAllen deleted the itertools branch September 6, 2017 19:34
ponylang-main added a commit that referenced this pull request Sep 6, 2017
@SeanTAllen SeanTAllen mentioned this pull request Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants