Use new compact parametric syntax where possible #20446

pabloferz · 2017-02-03T21:46:02Z

x-ref: #20414 (comment)

JeffBezanson · 2017-02-03T21:57:48Z

This is great. The payoff is especially large when multiple parameters are involved.

quinnj · 2017-02-03T22:00:37Z

base/linalg/cholesky.jl

@@ -249,7 +249,7 @@ end

 ### Default to no pivoting (and storing of upper factor) when not explicit
 cholfact!(A::Hermitian) = cholfact!(A, Val{false})
-cholfact!{T<:Real,S}(A::Symmetric{T,S}) = cholfact!(A, Val{false})
+cholfact!(A::Symmetric{<:Real}) = cholfact!(A, Val{false})


Can this really go from two parameters to one? I guess the 2nd isn't used at all?

julia> Symmetric{<:Real} Symmetric{#s1,S} where S<:(AbstractArray{T,2} where T) where #s1<:Real

And yes, the second isn't used.

tkelman · 2017-02-04T11:19:55Z

base/abstractarray.jl

@@ -89,7 +89,7 @@ julia> extrema(b)
 """
 linearindices(A)                 = (@_inline_meta; OneTo(_length(A)))
 linearindices(A::AbstractVector) = (@_inline_meta; indices1(A))
-eltype(::Type{A}) where A<:AbstractArray{E} where E  = E
+eltype(::Type{<:AbstractArray{E}}) where E  = E


parenthesize the where (E) ? would look a little more readable

tkelman · 2017-02-04T13:05:53Z

base/libgit2/index.jl

@@ -46,43 +46,43 @@ end
 read_tree!(idx::GitIndex, hash::AbstractGitHash) =
    read_tree!(idx, GitTree(repository(idx), hash))

-function add!{T<:AbstractString}(idx::GitIndex, files::T...;
-             flags::Cuint = Consts.INDEX_ADD_DEFAULT)
+function add!(idx::GitIndex, files::AbstractString...;


I think this should still work, but this change is slightly more permissive right? In the old version wouldn't each of files have to be the same type, but now they can individually be different AbstractString subtypes?

Sounds like an improvement.

tkelman · 2017-02-04T13:25:36Z

base/linalg/cholesky.jl

@@ -445,13 +445,13 @@ function getindex{T<:BlasFloat}(C::CholeskyPivoted{T}, d::Symbol)
    throw(KeyError(d))
 end

-show{T,S<:AbstractMatrix}(io::IO, C::Cholesky{T,S}) =
+show{T}(io::IO, C::Cholesky{T,<:AbstractMatrix}) =


could the T be <:Any here? (also L454)

Might be nice to support Foo{_,<:Bar} for this sort of thing eventually.

<:Any is perfectly clear to me, I don't think it needs an abbreviation

I agree that <:Any is clear, but I find @StevenG's

Foo{_,<:Bar}

cleaner than

Foo{<:Any,<:Bar}

Giving a special meaning to underscore like that is not at all intuitive. Saving 4 characters isn't worth ending up with code that requires finding an obscure manual section to understand.

tkelman · 2017-02-04T14:02:29Z

base/linalg/lu.jl

-A_ldiv_B!{T,S<:StridedMatrix}(A::LU{T,S}, B::StridedMatrix) = A_ldiv_B!(UpperTriangular(A.factors), A_ldiv_B!(UnitLowerTriangular(A.factors), B[ipiv2perm(A.ipiv, size(B, 1)),:]))
+A_ldiv_B!{T<:BlasFloat}(A::LU{T,<:StridedMatrix}, B::StridedVecOrMat{T}) = @assertnonsingular LAPACK.getrs!('N', A.factors, A.ipiv, B) A.info
+A_ldiv_B!{T}(A::LU{T,<:StridedMatrix}, b::StridedVector) = A_ldiv_B!(UpperTriangular(A.factors), A_ldiv_B!(UnitLowerTriangular(A.factors), b[ipiv2perm(A.ipiv, length(b))]))
+A_ldiv_B!{T}(A::LU{T,<:StridedMatrix}, B::StridedMatrix) = A_ldiv_B!(UpperTriangular(A.factors), A_ldiv_B!(UnitLowerTriangular(A.factors), B[ipiv2perm(A.ipiv, size(B, 1)),:]))


looks like T could be <:Any for the latter two

tkelman · 2017-02-04T14:05:26Z

base/linalg/lu.jl


-cond{T<:BlasFloat,S<:StridedMatrix}(A::LU{T,S}, p::Number) = inv(LAPACK.gecon!(p == 1 ? '1' : 'I', A.factors, norm((A[:L]*A[:U])[A[:p],:], p)))
+cond{T<:BlasFloat}(A::LU{T,<:StridedMatrix}, p::Number) = inv(LAPACK.gecon!(p == 1 ? '1' : 'I', A.factors, norm((A[:L]*A[:U])[A[:p],:], p)))


LU{<:BlasFloat,<:StridedMatrix} should work for these 3?

tkelman · 2017-02-04T14:10:18Z

base/linalg/symmetric.jl

 det(A::Symmetric) = det(bkfact(A))

-\{T,S<:StridedMatrix}(A::HermOrSym{T,S}, B::StridedVecOrMat) = \(bkfact(A.data, Symbol(A.uplo), issymmetric(A)), B)
+\{T}(A::HermOrSym{T,<:StridedMatrix}, B::StridedVecOrMat) = \(bkfact(A.data, Symbol(A.uplo), issymmetric(A)), B)


could T be replaced by <:Any here?

tkelman · 2017-02-04T14:17:56Z

base/linalg/triangular.jl

            LAPACK.trrfs!($uploc, 'N', $isunitc, A.data, B, X)

        # Condition numbers
-        function cond{T<:BlasFloat,S}(A::$t{T,S}, p::Real=2)
+        function cond{T<:BlasFloat}(A::$t{T}, p::Real=2)


A::$t{<:BlasFloat} ?

tkelman · 2017-02-04T14:20:17Z

base/linalg/triangular.jl

    LAPACK.trevc!('L', 'A', BlasInt[], tril!(A.data)')
 end
-function eigvecs{T<:BlasFloat,S<:StridedMatrix}(A::UnitLowerTriangular{T,S})
+function eigvecs{T<:BlasFloat}(A::UnitLowerTriangular{T,<:StridedMatrix})


A::XTriangular{<:BlasFloat,<:StridedMatrix} for these 4?

tkelman · 2017-02-04T14:22:39Z

base/sparse/abstractsparse.jl

+issparse{T}(S::LowerTriangular{T, <:AbstractSparseMatrix}) = true
+issparse{T}(S::LinAlg.UnitLowerTriangular{T, <:AbstractSparseMatrix}) = true
+issparse{T}(S::UpperTriangular{T, <:AbstractSparseMatrix}) = true
+issparse{T}(S::LinAlg.UnitUpperTriangular{T, <:AbstractSparseMatrix}) = true


{<:Any, <:AbstractSparseMatrix} for these?

tkelman · 2017-02-04T14:32:24Z

base/sparse/sparsematrix.jl

@@ -222,7 +222,7 @@ function reinterpret{T,Tv,Ti,N}(::Type{T}, a::SparseMatrixCSC{Tv,Ti}, dims::NTup
    return SparseMatrixCSC(mS, nS, colptr, rowval, nzval)
 end

-function copy{T,P<:SparseMatrixCSC}(ra::ReshapedArray{T,2,P})
+function copy{T}(ra::ReshapedArray{T,2,<:SparseMatrixCSC})


T can be <:Any (unless ReshapedArray has constraints I guess)

tkelman · 2017-02-04T14:41:29Z

base/sparse/sparsevector.jl

-hcat{Tv,Ti<:Integer}(X::SparseVector{Tv,Ti}...) = _absspvec_hcat(X...)
-hcat{Tv,Ti<:Integer}(X::AbstractSparseVector{Tv,Ti}...) = _absspvec_hcat(X...)
+hcat{Tv}(X::SparseVector{Tv,<:Integer}...) = _absspvec_hcat(X...)
+hcat{Tv}(X::AbstractSparseVector{Tv,<:Integer}...) = _absspvec_hcat(X...)


would this allow each argument to have a different index type?

Yes:

julia> f(::(Vector{T} where T<:Real)...) = 1 f (generic function with 1 method) julia> f([1,2,3], [4.0]) 1

stevengj · 2017-02-05T02:58:27Z

These lines can now probably be improved to:

rcum_promote_type{T,N}(op, ::Type{<:AbstractArray{T,N}}) = Array{rcum_promote_type(op,T), N}

pabloferz · 2017-02-05T06:01:03Z

These lines can now probably be improved

Done

tkelman · 2017-02-05T09:52:15Z

base/deprecated.jl

-@deprecate big{T<:Integer,N}(x::AbstractArray{Complex{Rational{T}},N}) big.(A)
+@deprecate big(A::AbstractArray{<:Complex{<:Integer}}) big.(A)
+@deprecate big(A::AbstractArray{<:Complex{<:AbstractFloat}}) big.(A)
+@deprecate big(x::AbstractArray{<:Complex{<:Rational{<:Integer}}}) big.(A)


why do these end up needing multiple levels of <: ?

An AbstractArray{Complex{<:Integer}} would be an array of elements that can hold any integer complex type (e.g. the elements can be Complex{Int}, Complex{Int8}, etcetera in the same array).

AbstractArray{Complex{<:Integer}} is equivalent to AbstractArray{Complex{T} where T<:Integer} so it matches AbstratctArrays with a UnionAll element type (Complex{T} where T<:Integer).

foo(::AbstractArray{Complex{<:Integer}}) = 1 bar(::AbstractArray{<:Complex{<:Integer}}) = 2 foo([1im]) #MethodError bar([1im]) == 2 foo((Complex{T} where T<:Integer)[1im]) == 1 bar((Complex{T} where T<:Integer)[1im]) == 2 # since AbstractArray{Complex{T} where T<:Integer} <: (AbstractArray{T} where T<:(Complex{I} where I))

UnionAll element types don't seem like they're going to occur in practice

Yeah, but AbstractArray{<:Complex{<:Integer}} will handle the cases we care about.

isn't the problem not the number of wheres we need, but the placement? so AbstractArray{Complex{<:Integer}} is AbstractArray{Complex{T} where T<:Integer} but we actually want AbstractArray{Complex{T}} where T<:Integer ?

We had R = AbstractArray{Complex{I},N} where N where I<:Integer and with this we have S = AbstractArray{C,N} where N where C<:(Complex{I} where I<:Integer), but R <: S. This will handle a little more cases (which we almost surely won't be seeing), but I don't see the harm in doing so.

tkelman · 2017-02-05T14:41:39Z

base/parallel/remotecall.jl

    f = deserialize_rr(s,t)
    Future(f.where, RRID(f.whence, f.id), f.v) # ctor adds to client_refs table
 end

-function deserialize{T<:RemoteChannel}(s::AbstractSerializer, t::Type{T})
+function deserialize(s::AbstractSerializer, t::Type{<:RemoteChannel})


you had also converted function deserialize(s::ClusterSerializer, t::Type{T}) where T <: CapturedException in an earlier version, which is now in clusterserialize.jl

Missed it with all the moving from that PR. Will change it again.

pabloferz changed the title ~~Use new compact parametric syntax where possible~~ WIP: Use new compact parametric syntax where possible Feb 3, 2017

andreasnoack approved these changes Feb 3, 2017

View reviewed changes

quinnj reviewed Feb 3, 2017

View reviewed changes

pabloferz force-pushed the pz/newparamsyntax branch 4 times, most recently from 786df30 to ee778f5 Compare February 4, 2017 11:07

tkelman reviewed Feb 4, 2017

View reviewed changes

pabloferz force-pushed the pz/newparamsyntax branch 2 times, most recently from fe86631 to 928c821 Compare February 4, 2017 11:34

tkelman reviewed Feb 4, 2017

View reviewed changes

pabloferz force-pushed the pz/newparamsyntax branch from 928c821 to 7195193 Compare February 4, 2017 21:08

pabloferz force-pushed the pz/newparamsyntax branch from 7195193 to 3d7d4bf Compare February 5, 2017 05:09

tkelman reviewed Feb 5, 2017

View reviewed changes

pabloferz force-pushed the pz/newparamsyntax branch from 3d7d4bf to 2965a38 Compare February 5, 2017 12:36

tkelman reviewed Feb 5, 2017

View reviewed changes

pabloferz force-pushed the pz/newparamsyntax branch 2 times, most recently from 66a7209 to 6a7b225 Compare February 6, 2017 20:39

pabloferz added 25 commits February 9, 2017 10:32

Use compact parametric syntax in base/libgit2

b04aa0f

Use compact parametric syntax in base/linalg

cc34b19

Use compact parametric syntax in base/markdown

f3e42c3

Use compact parametric syntax in base/parallel

35c453e

Use compact parametric syntax in base/pkg

aadd68c

Use compact parametric syntax in base/sparse

0f0ef4a

Use compact parametric syntax in base/strings

2ca4e64

Use compact parametric syntax in LineEdit.jl

8454b61

Use compact parametric syntax in base/a*.jl

f00a2eb

Use compact parametric syntax in base/b*.jl

bacc024

Use compact parametric syntax in base/c*.jl

038c8c5

Use compact parametric syntax in base/d*.jl

f7be095

Use compact parametric syntax in base/e*.jl

a3186b4

Use compact parametric syntax in base/f*.jl

3350ea8

Use compact parametric syntax in base/g*.jl

e0a18dd

Use compact parametric syntax in base/h*.jl

590cb53

Use compact parametric syntax in base/i*.jl

72d208b

Use compact parametric syntax in base/m*.jl

73dabe0

Use compact parametric syntax in base/n*.jl

84e0863

Use compact parametric syntax in base/o*.jl

5175b6b

Use compact parametric syntax in base/p*.jl

bbbfce0

Use compact parametric syntax in base/r*.jl

ec9505d

Use compact parametric syntax in base/s*.jl

3d8e7d8

Use compact parametric syntax in base/t*.jl

5989549

Use compact parametric syntax in base/u*.jl

82a63fd

pabloferz force-pushed the pz/newparamsyntax branch from 886da54 to 82a63fd Compare February 9, 2017 16:40

pabloferz mentioned this pull request Feb 9, 2017

Added ConjArray wrapper type for conjugate array views #20047

Merged

tkelman approved these changes Feb 9, 2017

View reviewed changes

pabloferz merged commit 79471c1 into JuliaLang:master Feb 9, 2017

pabloferz deleted the pz/newparamsyntax branch February 9, 2017 22:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use new compact parametric syntax where possible #20446

Use new compact parametric syntax where possible #20446

pabloferz commented Feb 3, 2017

JeffBezanson commented Feb 3, 2017

quinnj Feb 3, 2017

pabloferz Feb 3, 2017 •

edited

Loading

tkelman Feb 4, 2017

tkelman Feb 4, 2017

stevengj Feb 4, 2017

tkelman Feb 4, 2017 •

edited

Loading

stevengj Feb 4, 2017 •

edited

Loading

tkelman Feb 4, 2017

kmsquire Feb 5, 2017

tkelman Feb 5, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

tkelman Feb 4, 2017

stevengj Feb 4, 2017

stevengj commented Feb 5, 2017

pabloferz commented Feb 5, 2017

tkelman Feb 5, 2017

stevengj Feb 5, 2017

pabloferz Feb 5, 2017

tkelman Feb 5, 2017

pabloferz Feb 5, 2017 •

edited

Loading

tkelman Feb 5, 2017

pabloferz Feb 5, 2017

tkelman Feb 5, 2017

pabloferz Feb 5, 2017


		cond{T<:BlasFloat,S<:StridedMatrix}(A::LU{T,S}, p::Number) = inv(LAPACK.gecon!(p == 1 ? '1' : 'I', A.factors, norm((A[:L]*A[:U])[A[:p],:], p)))
		cond{T<:BlasFloat}(A::LU{T,<:StridedMatrix}, p::Number) = inv(LAPACK.gecon!(p == 1 ? '1' : 'I', A.factors, norm((A[:L]*A[:U])[A[:p],:], p)))

Use new compact parametric syntax where possible #20446

Use new compact parametric syntax where possible #20446

Conversation

pabloferz commented Feb 3, 2017

JeffBezanson commented Feb 3, 2017

Choose a reason for hiding this comment

pabloferz Feb 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman Feb 4, 2017 • edited Loading

Choose a reason for hiding this comment

stevengj Feb 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevengj commented Feb 5, 2017

pabloferz commented Feb 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Feb 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Feb 3, 2017 •

edited

Loading

tkelman Feb 4, 2017 •

edited

Loading

stevengj Feb 4, 2017 •

edited

Loading

pabloferz Feb 5, 2017 •

edited

Loading