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.

Sign up for GitHub

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

Jump to bottom

Disambiguate sub2ind #18352

Merged
merged 1 commit into from
Sep 7, 2016
Merged

Disambiguate sub2ind #18352

merged 1 commit into from
Sep 7, 2016
+24 −2

Conversation

timholy
Copy link
Member

@timholy timholy commented Sep 4, 2016

Also adds a broken test for #18307

tkelman
tkelman reviewed Sep 4, 2016
View reviewed changes
base/abstractarray.jl
@@ -1609,7 +1609,10 @@ _div(ind, d::Integer) = div(ind, d), 1, d
_div(ind, r::AbstractUnitRange) = (d = unsafe_length(r); (div(ind, d), first(r), d))

# Vectorized forms
function sub2ind{N,T<:Integer}(inds::Union{Dims{N},Indices{N}}, I::AbstractVector{T}...)
sub2ind{T<:Integer}(inds::Indices{1}, I1::AbstractVector{T}, I::AbstractVector{T}...) = throw(ArgumentError("Linear indexing is not defined for one-dimensional arrays"))
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap this?

@tkelman
Copy link
Contributor

tkelman commented Sep 4, 2016

Does @test_broken work properly with a @test_throws ?

@timholy
Copy link
Member Author

timholy commented Sep 5, 2016

Seems to, at least in this direction.

@tkelman
Copy link
Contributor

tkelman commented Sep 5, 2016

but will it serve the purpose of test_broken correctly? if something fixes the issue, will the test_broken flag it when it happens, or silently pass?

@vtjnash
Copy link
Member

vtjnash commented Sep 5, 2016

It'll silently pass, since @test_throws doesn't return true/false:

julia> Base.Test.@test_broken Base.Test.@test_throws AssertionError @assert true
Test Failed
  Expression: @assert true
    Expected: AssertionError
  No exception thrown
Test Broken
Expression: Base.Test.@test_throws AssertionError @assert(true)


julia> Base.Test.@test_broken Base.Test.@test_throws AssertionError @assert false
Test Broken
Expression: Base.Test.@test_throws AssertionError @assert(false)

@timholy
Copy link
Member Author

timholy commented Sep 5, 2016

Shall I just delete the test? Or comment it out?

@tkelman
Copy link
Contributor

tkelman commented Sep 5, 2016

don't use test_broken, but do check the current result with a comment that it's currently wrong and should be fixed

@timholy
Disambiguate sub2ind
e242d46 
Also adds a broken test for #18307
@timholy
Copy link
Member Author

timholy commented Sep 5, 2016

OK, see if this is better.

@vtjnash vtjnash merged commit e242d46 into master Sep 7, 2016
vtjnash added a commit that referenced this pull request Sep 7, 2016
@vtjnash
Merge pull request #18352 from 'JuliaLang/teh/sub2ind'
6732572 
Disambiguate sub2ind
@vtjnash vtjnash deleted the teh/sub2ind branch September 7, 2016 03:33
tkelman pushed a commit that referenced this pull request Sep 16, 2016
@timholy @tkelman
Disambiguate sub2ind
9ac7312 
Also adds a broken test for #18307

(cherry picked from commit e242d46)
ref #18352
@vtjnash vtjnash mentioned this pull request Feb 27, 2017
Closed
53 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@timholy @tkelman @vtjnash