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 length for zips with cycles #13276

Closed
wants to merge 2 commits into from
Closed

Conversation

mschauer
Copy link
Contributor

This fixes a small issue with Zips containing infinite components which prevented constructions like

[i for i in zip(1:10, cycle(1:2))]

@hayd
Copy link
Member

hayd commented Sep 22, 2015

An alternative to doing it this way is to define the length of Cycle and Repeated to be infinite:

Base.length(z::Base.Zip2) = Int(min(length(z.a), length(z.b)))   # change
Base.length(c::Base.Cycle) = Inf                                 # add
Base.length(r::Base.Repeated) = Inf                              # add

This would generalize but may be controversial (as length is otherwise an Int).

@mschauer
Copy link
Contributor Author

[edit: moved the argument over to #11977]

@ivarne
Copy link
Member

ivarne commented Sep 23, 2015

See also discussion in #11977

@mschauer
Copy link
Contributor Author

Sorry, @simonbyrne , there was no intention to pre-empt the introduction of Alephs into julia with this PR ;-)

@stevengj
Copy link
Member

You could also define the length of an infinite iterable to be typemax(Int), I suppose.

@mschauer
Copy link
Contributor Author

@stevengj My first reaction is that this would be not a good idea. Producing an error when asking for the length of an infinite object is the current convention. That also sounds really prone to produce overflow when doing innocent computations like n = length(l)+1.

@mschauer
Copy link
Contributor Author

#11977 is closed, can we go ahead here?


# Zips with infinite length components

length{I<:Union{Cycle,Repeated},Z<:AbstractZipIterator}(z::Zip{I,Z}) = length(z.z)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's about at the top we do

typealias InfiniteIterators Union{Cycle,Repeated}

and add a TODO that we should do this with traits once we have #13222.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good suggestion.

@mschauer
Copy link
Contributor Author

Can be done in a much nicer way with the new Iterator traits.

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 this pull request may close these issues.

5 participants