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

Use typealiases in method printing? #14946

Closed
jiahao opened this issue Feb 5, 2016 · 8 comments
Closed

Use typealiases in method printing? #14946

jiahao opened this issue Feb 5, 2016 · 8 comments
Labels
display and printing Aesthetics and correctness of printed representations of objects. speculative Whether the change will be implemented is speculative

Comments

@jiahao
Copy link
Member

jiahao commented Feb 5, 2016

I'd like to request some speculative brain cells spent on how we print method signatures to the REPL.

Currently, we expand out all typealiases. While this policy has the advantage of producing unambiguous output, the composition of type aliases through type parameters leads to combinatorially long string representations.

A particularly egregious example in Base is eigfact!:

julia> methods(eigfact!)

#12 methods for generic function "eigfact!":

...

eigfact!{T<:Union{Complex{Float32},Complex{Float64}}}(A::Union{DenseArray{T,2},SubArray{T,2,A<:DenseArray{T<:Any,N<:Any},I<:Tuple{Vararg{Union{Colon,Int64,Range{Int64}}}},LD<:Any}}) at linalg/eigen.jl:50

eigfact!{T<:Union{Float32,Float64}}(A::Union{DenseArray{T,2},SubArray{T,2,A<:DenseArray{T<:Any,N<:Any},I<:Tuple{Vararg{Union{Colon,Int64,Range{Int64}}}},LD<:Any}}, B::Union{DenseArray{T,2},SubArray{T,2,A<:DenseArray{T<:Any,N<:Any},I<:Tuple{Vararg{Union{Colon,Int64,Range{Int64}}}},LD<:Any}}) at linalg/eigen.jl:121

eigfact!{T<:Union{Complex{Float32},Complex{Float64}}}(A::Union{DenseArray{T,2},SubArray{T,2,A<:DenseArray{T<:Any,N<:Any},I<:Tuple{Vararg{Union{Colon,Int64,Range{Int64}}}},LD<:Any}}, B::Union{DenseArray{T,2},SubArray{T,2,A<:DenseArray{T<:Any,N<:Any},I<:Tuple{Vararg{Union{Colon,Int64,Range{Int64}}}},LD<:Any}}) at linalg/eigen.jl:142

...

I estimate that only three people in the world can read the first method signature and immediately recognize that the original calling signature was

eigfact!{T<:BlasComplex}(A::StridedMatrix{T})

and it would take an eagle eye to realize that the first method takes one argument whereas the other two take two inputs each.

It would be much nicer to print something like the original method signature instead of feeling overwhelmed by C++ expression template-like verbal diarrhea. It's clear, though, that the additional bookkeeping of typealiases and matching of typealiases to method signatures is a hard problem.

@carnaval
Copy link
Contributor

carnaval commented Feb 5, 2016

a couple of things that would make this one slightly less horrible but are much easier to do right now :

  • avoid printing type upper bounds of type vars when it's Any
  • put back some syntax support for tuple types and varargs inside them

the general problem of which type alias to pick for a given expanded type is as you say much harder

@carnaval
Copy link
Contributor

carnaval commented Feb 5, 2016

an infix version (using unicode ?) of Union{} would also help but I feel this will be more controversial

@quinnj
Copy link
Member

quinnj commented Feb 5, 2016

+1. I've thought/wanted this several times. Right now, it seems that the typealias expansion happens immediately; I wonder if it would make sense or even be possible to do the expanding at a later time, like only for compilation or something.

@simonster
Copy link
Member

I think there is a special case in the show code for printing ByteString for this reason. Just having this for StridedArray would be great, but obviously a more general solution would be better.

@jiahao jiahao added the speculative Whether the change will be implemented is speculative label Feb 5, 2016
@IainNZ
Copy link
Member

IainNZ commented Feb 5, 2016

This is a dream of mine too, as it would make JuMP errors much more succinct

@StefanKarpinski
Copy link
Member

For simple (non-parametric) type aliases, we could just have a hash table from the expanded type back to the alias name. But for parametric types it's not so simple. There's also the issue that the same type can have various different names and we could end up printing a different/confusing one. The only approach that seems like it would do what one expects is to remember how the type name was spelled at method definition and use that specific spelling when printing things.

@timholy
Copy link
Member

timholy commented Jan 24, 2020

Worth pointing out that saving some variant of the source text would be an alternative way of solving this. Demo:

julia> using LinearAlgebra, CodeTracking, Revise

julia> Revise.track(LinearAlgebra)

julia> m = @which eigen!(rand(3,3));

julia> ex = definition(m);

julia> ex.args[end].args[1]
:(eigen!(A::StridedMatrix{T}; permute::Bool=true, scale::Bool=true, sortby::Union{Function, Nothing}=eigsortby) where T <: BlasReal)

@simeonschaub
Copy link
Member

I think this was fixed by #36107.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. speculative Whether the change will be implemented is speculative
Projects
None yet
Development

No branches or pull requests

9 participants