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

2-arg show for GPUArray #429

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

2-arg show for GPUArray #429

wants to merge 4 commits into from

Conversation

mcabbott
Copy link
Contributor

Replaces JuliaGPU/CUDA.jl#1197 with a more generic version:

julia> using JLArrays

julia> (x=[1.0], y=[2f0 3f0])  # Arrays
(x = [1.0], y = Float32[2.0 3.0])

julia> map(jl, ans)  # previously looked identical
(x = JLArray([1.0]), y = JLArray(Float32[2.0 3.0]))

This sort-of assumes typeof(A).name.name is a reasonable constructor.

Unlike JuliaGPU/CUDA.jl#1197, this is going to print CuArray(Float32[2.0 3.0]) rather than shortening to cu([2.0 3.0]).

@@ -283,13 +283,13 @@ end
# due to different definition of `Int` type
# print([1]) shows as [1] on 64bit but Int64[1] on 32bit
msg = showstr(A)
@test msg == "[1]" || msg == "Int64[1]"
@test occursin("[1]", msg) || occursin("Int64[1]", msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple occursins with strings that are substrings of each other isn't very useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true. Maybe these should detect the brackets which this PR adds, at least this will fail before:

Suggested change
@test occursin("[1]", msg) || occursin("Int64[1]", msg)
@test occursin("([1])", msg) || occursin("(Int64[1])", msg)

Copy link
Contributor Author

@mcabbott mcabbott Sep 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests fail on CI, and running the package's tests locally, on Julia <= 1.8.

But what they aim to test seems to work fine in isolation, and with just this @testset. Any idea what's wrong?

julia> using JLArrays

julia> jl([1]), 2  # this works fine:
(JLArray([1]), 2)

julia> # from tests:
       replstr(x, kv::Pair...) = sprint((io,x) -> show(IOContext(io, :compact => false, :limit => true, :displaysize => (24, 80), kv...), MIME("text/plain"), x), x)
       showstr(x, kv::Pair...) = sprint((io,x) -> show(IOContext(io, :limit => true, :displaysize => (24, 80), kv...), x), x);

julia> replstr(jl([1]))
"1-element JLArray{Int64, 1}:\n 1"

julia> showstr(jl([1]))  # still fine
"JLArray([1])"

julia> let AT = JLArray
               @testset "showing" begin
                   # vectors and non-vector arrays showing
                   # are handled differently in base/arrayshow.jl
                   A = AT(Int64[1])
                   B = AT(Int64[1 2;3 4])
                   msg = replstr(A)
                   @test occursin(Regex("^1-element $AT{Int64,\\s?1.*}:\n 1\$"), msg)
                   # # result of e.g. `print` differs on 32bit and 64bit machines
                    # due to different definition of `Int` type
                    # print([1]) shows as [1] on 64bit but Int64[1] on 32bit
                    msg = showstr(A)
                    @test occursin("([1])", msg) || occursin("(Int64[1])", msg)

                    msg = replstr(B)
                    @test occursin(Regex("^2×2 $AT{Int64,\\s?2.*}:\n 1  2\n 3  4\$"), msg)

                    msg = showstr(B)
                    @test occursin("([1 2; 3 4])", msg) || occursin("(Int64[1 2; 3 4])", msg)

                    # the printing of Adjoint depends on global state
                    msg = replstr(A')
                   @test occursin(Regex("^1×1 Adjoint{Int64,\\s?$AT{Int64,\\s?1.*}}:\n 1\$"), msg) ||
                       occursin(Regex("^1×1 LinearAlgebra.Adjoint{Int64,\\s?$AT{Int64,\\s?1.*}}:\n 1\$"), msg) ||
                       occursin(Regex("^1×1 adjoint\\(::$AT{Int64,\\s?1.*}\\) with eltype Int64:\n 1\$"), msg)
               end
           end
Test Summary: | Pass  Total
showing       |    5      5
Test.DefaultTestSet("showing", Any[], 5, false, false)

julia> VERSION
v"1.6.0"

test/testsuite/base.jl Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants