Flux type piracy breaks REPL completions

[KristofferC]

> Base.show(<TAB>┌ Error: Error in the keymap
│   exception =
│    UndefVarError: T not defined
│    Stacktrace:
│     [1] show(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::Type{Flux.Data.Tree{T}}) at C:\Users\Kristoffer\MachineLearning\dev\Flux\src\data\tree.jl:17
│     [2] show_datatype(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::DataType) at .\show.jl:526
│     [3] show(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::DataType) at .\show.jl:436
│     [4] print(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::Type) at .\strings\io.jl:31
│     [5] string_with_env(::Base.ImmutableDict{Symbol,Any}, ::Type) at .\strings\io.jl:140
│     [6] argtype_decl(::Base.ImmutableDict{Symbol,Any}, ::Symbol, ::DataType, ::Int64, ::Int32,
::Bool) at .\methodshow.jl:42
│     [7] arg_decl_parts(::Method) at .\methodshow.jl:69
│     [8] #show#520(::DataType, ::Function, ::Base.GenericIOBuffer{Array{UInt8,1}}, ::Method) at
.\methodshow.jl:113

Type piracy is here:

Flux.jl/src/data/tree.jl

Lines 16 to 17 in 78876a1

	Base.show(io::IO, t::Type{Tree}) = print(io, "Tree")
	Base.show(io::IO, t::Type{Tree{T}}) where T = print(io, "Tree{", T, "}")

[MikeInnes]

Not exactly type piracy since we own the type Tree. But it'd be good to fix the method by checking @isdefined anyhow. Or maybe there's a simpler way to avoid the Flux.Data.Tree{T} printing.

[KristofferC]

It is type piracy since you don't own Type (JuliaLang/julia#22363 (comment)).

[MikeInnes]

Usually the significance of type piracy is that it can change the behaviour of unrelated code. That's not generally the case when you define something like show(io, ::Array{Foo}), since that method couldn't have been called without the Foos package loaded.

Showing a type is definitely a special case because it's used by reflection in e.g. tab completion and so will inevitably change when a package is loaded. But that's an unusual property of that function specifically. You would get breakage if you did something like define a billion methods for show, even with no type piracy involved.

[Saying it's not type piracy isn't the same as saying it's not bad to do, I'm just splitting hairs between the mechanism and the end result.]

[AzamatB]

Bump. This shows up in non-obvious places like when calculating gradient:

julia> l, pb = Flux.Zygote.pullback(θ) do
          loss(xs, ys)
       end;
ERROR: UndefVarError: S not defined
Stacktrace:
 [1] (::Type{ERROR: UndefVarError: S not defined
Stacktrace:
 [1] show(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::Type{ERROR: UndefVarError: S not defined
Stacktrace:
 [1] show(::IOContext{Base.TTY}, ::Type{
SYSTEM (REPL): showing an error caused an error
ERROR: UndefVarError: S not defined
Stacktrace:
 [1] show(::IOContext{REPL.Terminals.TTYTerminal}, ::Type{
SYSTEM (REPL): caught exception of type UndefVarError while trying to handle a nested exception; giving up

[MikeInnes]

This isn't quite the same error, since it's not coming from the show method but from a perfectly non-type-piratical constructor (in the cases of it that I've seen). See FluxML/Zygote.jl#368 for that.

[darsnack]

@CarloLucibello this will become a non-issue once the datasets are moved out, right? Does it make sense to add a milestone tag or something remind us to close it when that happens?