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

RFC: Make Tuple types with defined lengths iterable #11547

Closed
wants to merge 2 commits into from
Closed

Conversation

mbauman
Copy link
Member

@mbauman mbauman commented Jun 2, 2015

More abstract tuple types like Tuple or NTuple (without parameters) and vararg tuples remain without any iteration methods defined.

This is something I'd really like to see, but I understand that it may be contentious. For the most part this is fairly straightforward, but I did discover one minor confusion stemming from a definition for eltype for NTuples of all the same type. Since eltype works in the type domain, there is no way to tell whether eltype{N,T}(::Type{NTuple{N,T}}) should be T or Type{T}. I have removed these definitions. This broke one unrelated test, since collect((1,2,3)) is now Any[1,2,3], and indexing with an Any array isn't implemented.

More abstract tuple types like `Tuple` or `NTuple` (without parameters) and vararg tuples remain without any iteration methods defined.

I only discovered one minor confusion stemming from a definition for `eltype` for NTuples of all the same type.  Since eltype works in the type domain, there is no way to tell whether `eltype{N,T}(::Type{NTuple{N,T}})` should be `T` or `Type{T}`. I have removed these definitions.
@mauro3
Copy link
Contributor

mauro3 commented Jun 3, 2015

Yes please! It's crazy to have to use .parameters to iterate over Tuples. (Also, why is it contentious? It has been mentioned a few times, and it seems like people-who-know know why. I don't belong to that group...)

@StefanKarpinski
Copy link
Member

This seems sensible to me as well – @JeffBezanson, do you have a reason not to do this?

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

One thing I'd still like to test is this PR plus @timholy's #11242. This leans heavily on NTuple, and I really don't want to put any roadblocks in the way of #11242. I'll give that a shot later today.

@timholy
Copy link
Member

timholy commented Jun 3, 2015

Would be great to test, but even with #11242 NTuple still exists, it's just no longer a builtin type. So in theory it should be fine.

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

Yup, this should be good to go. There's a very simple solution (adb9a14) to the issues I had with #11242 if they don't get solved there. 👍

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

Ah, and there will have to be special-casing for iterating over Tuple{…, Vararg{T,N}} (edit: or maybe #11242 should expand them out upon construction like NTuple):

julia> collect(Tuple{Vararg{Int,3}})
3-element Array{Any,1}:
 Int64
 Int64
 Int64

julia> collect(Tuple{String,Vararg{Int,3}})
4-element Array{Any,1}: # the length is correct
    AbstractString
    Vararg{Int64,3}
 #undef
 #undef

@JeffBezanson
Copy link
Member

There shouldn't be any need to remove eltype. Tuples and tuple types are completely disjoint. eltype(Tuple{Int,Int}) means the element type of values of the given type. If the type itself is iterable, you need to ask for eltype(Type{Tuple{Int,Int}}).

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

The trouble is that we define eltype(x) = eltype(typeof(x)) and support asking either the object or the type. Is eltype(Tuple{Int,Int}) referring to the Type or the Tuple?

@JeffBezanson
Copy link
Member

Well, that definition only applies to non-Types.

Anyway I have much more serious objections to this. This depends on isa(Tuple{Int,String}, Type{NTuple{N}}), which as far as I can tell is completely wrong. This seems to be exploiting a bug in the type system.

This kind of approach will increasingly be the wrong way to operate on types. Vararg{T,3} is one example. What you really want is to perform certain computations in the type domain. The right way to "index" a tuple type is basically getfield_tfunc(tuple_type, Constant{i}) (that's not its current signature but you get the idea). That code would automatically handle Vararg, and even exotic things like Unions of tuple types. This is much more general. There are other type-domain computations on tuples we want, like (T1..., T2...) (concatenating tuple types).

This is basically the same situation that we have in arithmetic and linear algebra, where it was proposed to add definitions like

+(::Type{Int}, ::Type{Int}) = Int

It should be clear how that does not scale.

Put differently, it's not such a big win to provide only iteration. Many use cases want indexing, or concatenation, etc.

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

Alright, see my take 2. All computations are done in the type domain now, and I've eliminated the funky dispatch on ::Type{Tuple{N}}.

I disagree that iteration isn't a big win. Right now it's very difficult for users to dig into the contents of Tuple{…}. This allows access in an idiomatic fashion and trivially enables concatenation Tuple{A..., B...}. For my use-cases, it's totally sufficient.

What are your thoughts on merging an implementation like this?

@JeffBezanson
Copy link
Member

This is better than the last version, though the staged functions don't seem to be necessary.

I just still find it problematic to have types implement value-level interfaces. eltype is the most obvious symptom: this is a totally ordinary, innocent function, and here we have code that's only needed to assist metaprogramming conflicting with it.

This also has the problem described in issue #11164.

@JeffBezanson
Copy link
Member

FWIW, I also dislike that Enum types are iterable. I'd rather have something like for x in instances(enumtype).

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

Alright. So, what's the path forward here? Refactoring the interface to Core.Inference.getfield_tfunc? A helper function that takes a Tuple{A,B,C} and returns (A,B,C)?

@JeffBezanson
Copy link
Member

Well, x->x.parameters already gives you an iterable, indexable, etc. view of the tuple type elements. While this has been described above as "crazy", manipulating types is already pretty crazy. Maybe the name is just too long. Should we call it x.p? x.elts?

@carnaval
Copy link
Contributor

carnaval commented Jun 3, 2015

The "good" thing about .parameters is that at least you have this feeling that you have to be careful because you're touching internal representation, which is the case.

Manipulating tuple types correctly is hard for anything but fixed length leaf ones.

@mbauman
Copy link
Member Author

mbauman commented Jun 3, 2015

you have this feeling that you have to be careful because you're touching internal representation

That's precisely what I want to fix! It seems very valuable to have some sort of blessed method that only works in well-defined scenarios (and has sensible error messages otherwise)… especially as the internal representations become more complicated with Vararg{T,N} or other refactorings.

The behavior of Tuple types is well-defined, and its type parameters are part of that well-defined external interface. "Go digging around in .parameters and sort it out yourself" is a very unsatisfactory resolution here.

@ScottPJones
Copy link
Contributor

FWIW, I agree totally with @mbauman. Having people play around with internal representation all over the place goes against everything I ever learned about good software engineering practices. What's worse, I believe that with Julia, it doesn't even gain you any performance benefits.
All you have to do is define the interface for the internals of your type, with little one-line functions usually, and Julia will do a great job of inlining things so that something like parameters(xxx) and xxx.parameters generate the exact same code (but you could later change the internal representation, as long as you kept the interface of parameters(xxx) the same!

@JeffBezanson
Copy link
Member

I have no problem with wrapping this in an API, e.g. tparam(t, i) for t.parameters[i]. That's not the issue. In fact there are two separate functions we need. One directly inspects the representation, e.g. telling you that Vararg{Int} is the 2nd element of Tuple{T, Vararg{Int}}, and the other is type-domain indexing, telling you what the type of the nth element of a value of the given type would be. All the more reason to design an API just for this purpose instead of using iteration.

@ScottPJones
Copy link
Contributor

👍 to @JeffBezanson last comment, (IMO there are a lot of places in Julia where wrapping in well-designed API would be quite beneficial, and not cost any performance because of the great way Julia works)

@mbauman
Copy link
Member Author

mbauman commented Jun 4, 2015

I have no problem with wrapping this in an API

Ah, good. That's what I was after. Closing this in favor of a different API, then.

@mbauman
Copy link
Member Author

mbauman commented Jun 6, 2015

For those following along at home, I've created a test-bed repository to hash out the interface at TupleTypes.jl. This doesn't need to be defined in Base to work, but I think it'd be good to eventually move it back in since I think it should be an official API.

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.

7 participants