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

Make StridedArray display as StridedArray #31149

Merged
merged 1 commit into from
May 1, 2020
Merged

Make StridedArray display as StridedArray #31149

merged 1 commit into from
May 1, 2020

Conversation

mbauman
Copy link
Member

@mbauman mbauman commented Feb 22, 2019

Instead of a huge union. This is just a special-casing for this particular union instead of pursuing the general (and hard!) typealias search. But given the ubiquity of StridedArray in method lists, I find this special case to be warranted and hugely beneficial. Here is a sampling from methods(*):

[242] *(A::LinearAlgebra.AbstractQ, b::StridedArray{T, 1} where T) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:556
[243] *(A::LinearAlgebra.AbstractQ, B::StridedArray{T, 2} where T) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:568
[244] *(Q::LinearAlgebra.AbstractQ, adjB::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:(StridedVecOrMat{T} where T) where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:623
[245] *(A::StridedArray{T, 2} where T, Q::LinearAlgebra.AbstractQ) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:668
[246] *(A::StridedArray{T, 2} where T, adjB::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:LinearAlgebra.AbstractQ where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:708
[247] *(A::LinearAlgebra.LQPackedQ, B::StridedVecOrMat{T} where T) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:153
[248] *(A::LinearAlgebra.LQPackedQ, adjB::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:(StridedVecOrMat{T} where T) where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:177
[249] *(A::StridedVecOrMat{T} where T, adjQ::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:LinearAlgebra.LQPackedQ where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:216
[250] *(A::StridedVecOrMat{T} where T, Q::LinearAlgebra.LQPackedQ) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:240

@mbauman mbauman added arrays [a, r, r, a, y, s] display and printing Aesthetics and correctness of printed representations of objects. labels Feb 22, 2019
@mbauman
Copy link
Member Author

mbauman commented Feb 22, 2019

Interesting — the AppVeyor failures are actually real here. I'll have to scrounge up a windows VM to figure out what's going on there. The failure is startswith(sprint(show, StridedVecOrMat), "StridedVecOrMat").

Travis Mac is a network connectivity timeout.

@mbauman
Copy link
Member Author

mbauman commented Feb 26, 2019

Alright, now the AppVeyor failures are unrelated.

@StefanKarpinski
Copy link
Member

Let's do this! I propose merging if there are no objections by triage.

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Mar 18, 2019
base/show.jl Show resolved Hide resolved
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Mar 28, 2019
base/show.jl Outdated Show resolved Hide resolved
@StefanKarpinski
Copy link
Member

Triage approves this as a bandaid until we have a more general solution to the excessively verbose printing of complex types.

@StefanKarpinski StefanKarpinski added the minor change Marginal behavior change acceptable for a minor release label Mar 28, 2019
@StefanKarpinski
Copy link
Member

NEWSworthy?

@vtjnash
Copy link
Member

vtjnash commented Mar 28, 2019

Can/should this use showarg instead?

@mbauman
Copy link
Member Author

mbauman commented Mar 28, 2019

Oh cool. I had forgotten about showarg. Yes, that looks to accomplish what I want without making this totally obscure what a StridedArray is.

@mbauman
Copy link
Member Author

mbauman commented Mar 28, 2019

Oh, nope, showarg isn't going to do the trick. It works in the value domain, not in the type domain.

@mbauman
Copy link
Member Author

mbauman commented Apr 28, 2020

Rebased, fixed up, and I further added support for Union{StridedArray, SomethingElse} because it was easy to do and I noticed the crazy StridedOrTriangularMatrix Union in methods(*):

const StridedOrTriangularMatrix{T} = Union{StridedMatrix{T}, LowerTriangular{T}, UnitLowerTriangular{T}, UpperTriangular{T}, UnitUpperTriangular{T}}

Instead of a huge union.  This is just a special-casing for this particular union instead of pursuing the general (and hard\!) typealias search.  But given the ubiquity of StridedArray in method lists, I find this special case to be warranted and hugely beneficial. Here is a sampling from `methods(*)`:

```julia
[242] *(A::LinearAlgebra.AbstractQ, b::StridedArray{T, 1} where T) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:556
[243] *(A::LinearAlgebra.AbstractQ, B::StridedArray{T, 2} where T) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:568
[244] *(Q::LinearAlgebra.AbstractQ, adjB::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:(StridedVecOrMat{T} where T) where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:623
[245] *(A::StridedArray{T, 2} where T, Q::LinearAlgebra.AbstractQ) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:668
[246] *(A::StridedArray{T, 2} where T, adjB::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:LinearAlgebra.AbstractQ where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/qr.jl:708
[247] *(A::LinearAlgebra.LQPackedQ, B::StridedVecOrMat{T} where T) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:153
[248] *(A::LinearAlgebra.LQPackedQ, adjB::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:(StridedVecOrMat{T} where T) where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:177
[249] *(A::StridedVecOrMat{T} where T, adjQ::LinearAlgebra.Adjoint{#s614,#s613} where #s613<:LinearAlgebra.LQPackedQ where #s614) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:216
[250] *(A::StridedVecOrMat{T} where T, Q::LinearAlgebra.LQPackedQ) in LinearAlgebra at /home/mbauman/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/lq.jl:240
```
@mbauman mbauman merged commit 7583d87 into master May 1, 2020
@mbauman mbauman deleted the mb/sickofstrided branch May 1, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] display and printing Aesthetics and correctness of printed representations of objects. minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants