-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Some fixes for summary, show, and map for non-1 indices #17729
Conversation
@@ -195,6 +194,9 @@ show(io, v) | |||
str = takebuf_string(io) | |||
show(io, parent(v)) | |||
@test str == takebuf_string(io) | |||
smry = summary(v) | |||
@test contains(smry, "OffsetArray{Float64,1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing }
omission intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The third parameter is the "parent" array type, and I don't care about that here.
@@ -40,7 +40,7 @@ indices{T,N}(A::AbstractArray{T,N}, d) = d <= N ? indices(A)[d] : OneTo(1) | |||
|
|||
Returns the tuple of valid indices for array `A`. | |||
""" | |||
function indices{T,N}(A::AbstractArray{T,N}) | |||
function indices(A) | |||
@_inline_pure_meta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't pure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I should take the pure out of the fallback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, incidentally "fixed" #17126, that was unexpected.
for n = 0:4 | ||
a = OffsetArray(rand(ntuple(d->3,n)), ntuple(identity,n)) | ||
show(IOContext(io, limit=true), MIME("text/plain"), a) | ||
show(IOContext(io, limit=true), MIME("text/plain"), (a,a)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the outputs here be tested?
Seem OK now? |
are you about to push something? should probably read from |
6001540
to
c3e71d4
Compare
@@ -207,6 +209,23 @@ cmp_showf(Base.print_matrix, io, OffsetArray(rand(5,5), (10,-9))) # rows&c | |||
cmp_showf(Base.print_matrix, io, OffsetArray(rand(10^3,5), (10,-9))) # columns fit | |||
cmp_showf(Base.print_matrix, io, OffsetArray(rand(5,10^3), (10,-9))) # rows fit | |||
cmp_showf(Base.print_matrix, io, OffsetArray(rand(10^3,10^3), (10,-9))) # neither fits | |||
targets1 = ["0-dimensional OAs.OffsetArray{Int64,0,Array{Int64,0}}:\n1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this need to be $Int
on 32 bit systems?
c3e71d4
to
81c1b0b
Compare
|
Previously I specialized this in the tests, but it's better to just make this do the right thing out of the box.