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

In v2, Iterators.peel could return nothing when iterator is empty #39569

Closed
cmcaine opened this issue Feb 8, 2021 · 4 comments · Fixed by #39607
Closed

In v2, Iterators.peel could return nothing when iterator is empty #39569

cmcaine opened this issue Feb 8, 2021 · 4 comments · Fixed by #39607

Comments

@cmcaine
Copy link
Contributor

cmcaine commented Feb 8, 2021

Iterators.peel is a lazy version of h, t... = collection, which is a very common pattern in recursive code. I think it would be good if it were more ergonomic to use. If h, t... = collection will still be eager in v2, I think the simplest improvement would be to have Iterators.peel pass through nothing if the iterator is empty so that we don't need to do exception handling.

Current behaviour:

try
    h, t = Iterators.peel(itr)
    # do something with h and t
catch e
    if e isa BoundsError
        # itr was probably empty
    else
        rethrow()
    end
end

New proposal:

x = Iterators.peel(itr)
if isnothing(x)
    # itr was empty
end
h, t = x
# do something with h, t
@vtjnash
Copy link
Sponsor Member

vtjnash commented Feb 10, 2021

Care to make a PR? Looks like it should be non-breaking, since an attempt later to index or splat that nothing would still fail.

@cmcaine
Copy link
Contributor Author

cmcaine commented Feb 10, 2021

Sure. If for some reason you do iterators.peel and pass the object along without looking at it, this would move the location of the exception quite a lot, but if that's okay I can do a PR.

cmcaine added a commit to cmcaine/julia that referenced this issue Feb 11, 2021
Fixes JuliaLang#39569.

Old behaviour:
```jl
try
    h, t = Iterators.peel(itr)
    # do something with h and t
catch e
    if e isa BoundsError
        # itr was probably empty
    else
        rethrow()
    end
end
```

New behaviour:
```jl
x = Iterators.peel(itr)
if isnothing(x)
    # itr was empty
end
h, t = x
```
cmcaine added a commit to cmcaine/julia that referenced this issue Feb 11, 2021
Fixes JuliaLang#39569.

Old behaviour:
```jl
try
    h, t = Iterators.peel(itr)
    # do something with h and t
catch e
    if e isa BoundsError
        # itr was probably empty
    else
        rethrow()
    end
end
```

New behaviour:
```jl
x = Iterators.peel(itr)
if isnothing(x)
    # itr was empty
end
h, t = x
```
@cmcaine
Copy link
Contributor Author

cmcaine commented Feb 11, 2021

Submitted a PR. It passes tests, but I think the change should probably be run against the wider ecosystem.

@cmcaine
Copy link
Contributor Author

cmcaine commented Feb 11, 2021

Just did a code search on GitHub, and none of the 140 matches for Iterators.peel would be broken by this change.

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 a pull request may close this issue.

2 participants