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

inconsistent handling of abstract types in NamedTuple #34547

Closed
stevengj opened this issue Jan 28, 2020 · 5 comments
Closed

inconsistent handling of abstract types in NamedTuple #34547

stevengj opened this issue Jan 28, 2020 · 5 comments
Labels
speculative Whether the change will be implemented is speculative types and dispatch Types, subtyping and method dispatch

Comments

@stevengj
Copy link
Member

stevengj commented Jan 28, 2020

I noticed the following seeming inconsistency:

julia> (3,4) isa Tuple{Number,Number}
true

julia> (a=3,b=4) isa NamedTuple{(:a,:b), Tuple{Number,Number}}
false

You can use <:Tuple for the second argument for subtyping, but even that doesn't work as a constructor:

julia> (a=3,b=4) isa NamedTuple{(:a,:b), <:Tuple{Number,Number}}
true

julia> NamedTuple{(:a,:b), <:Tuple{Number,Number}}((3,4))
ERROR: MethodError: no method matching NamedTuple{(:a, :b),#s1} where #s1<:Tuple{Number,Number}(::Tuple{Int64,Int64})
Stacktrace:
 [1] top-level scope at REPL[9]:1

(Of course, Tuple{Number,Number}(3,4) doesn't work either. Correction: Tuple{Number,Number}((3,4)) is the correct spelling and works.)

This makes it a bit hard to use a NamedTuple declaration with abstract types for the fields. Proposal:

  1. (a=3,b=4) isa NamedTuple{(:a,:b), Tuple{Number,Number}} should return true, and in general it should work like isa Tuple{...}.

  2. Tuple{Number,Number} and NamedTuple{(:a,:b), Tuple{Number,Number}} should also work as constructors, taking the concrete type from their argument types.

In theory these are breaking, but in practice I think it should qualify as a minor change. NamedTuple with abstract types is basically useless right now, and (2) throws an exception, so it is hard to imagine any working code depending on the current behaviors.

@stevengj stevengj added speculative Whether the change will be implemented is speculative types and dispatch Types, subtyping and method dispatch labels Jan 28, 2020
@quinnj
Copy link
Member

quinnj commented Jan 28, 2020

Ref #32699, which is about making abstractly inferred NamedTuples useful

@JeffBezanson
Copy link
Member

Making NamedTuple covariant has been discussed before, and the current thinking is that it's probably a bad idea. In fact the opposite change, making tuples invariant, is on the table: #24614. For tabular data, we want a named tuple with a Union{Int,Missing} element type to be concrete.

The Tuple constructor already exists:

julia> Tuple{Number,Number}((1,2))
(1, 2)

julia> typeof(ans)
Tuple{Int64,Int64}

The NamedTuple constructor behaves as we need:

julia> NamedTuple{(:a,), Tuple{Union{Int,Missing}}}((1,))
NamedTuple{(:a,),Tuple{Union{Missing, Int64}}}((1,))

i.e. if you request a NamedTuple with abstract element types you get one with that exact (concrete) type. This is not useless, but in fact essential for tabular data with missings.

@stevengj
Copy link
Member Author

Whoops, I forgot the parentheses in the constructor invocation! You're correct that this works with abstract types, but not with <:Tuple types. Shouldn't this work?

julia> NamedTuple{(:a,:b), <:Tuple{Number,Number}}((3,4))
ERROR: MethodError: no method matching NamedTuple{(:a, :b),#s1} where #s1<:Tuple{Number,Number}(::Tuple{Int64,Int64})

similar to this which already works:

julia> NamedTuple{(:a,:b), <:Tuple}((3,4))
(a = 3, b = 4)

?

But we should be able to make NamedTuple{(:a,:b), <:Tuple{Number,Number}}(::Tuple) work just by adding a method, I guess.

@JeffBezanson
Copy link
Member

NamedTuple{(:a,:b), <:Tuple}((3,4)) happens to work since it's the same as NamedTuple{(:a,:b)}((3,4)). Handling a more specific upper bound is a little trickier, since it's less clear what type to return. I guess we could try calling NamedTuple{(:a,:b)}(arg), and then just assert that it matches the type you requested?

@stevengj
Copy link
Member Author

Anyway, closing this issue since it looks like making NamedTuple covariant is unlikely to happen, and constructors for abstract NamedTuple fields work okay:

julia> NamedTuple{(:x,:y), Tuple{Integer,Number}}((x=3,y=4))
NamedTuple{(:x, :y),Tuple{Integer,Number}}((3, 4))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
speculative Whether the change will be implemented is speculative types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

No branches or pull requests

3 participants