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

Warnings of + and - #7294

Closed
lindahua opened this issue Jun 17, 2014 · 8 comments
Closed

Warnings of + and - #7294

lindahua opened this issue Jun 17, 2014 · 8 comments

Comments

@lindahua
Copy link
Contributor

I just checked out the latest master, and when I was watching the repo being rebuilt, I saw the following warnings:

Warning: New definition 
    + could not show value of type Tuple at array.jl:771
is ambiguous with: 
    + could not show value of type Tuple at range.jl:428.
To fix, define 
    + could not show value of type Tuple
before the new definition.
Warning: New definition 
    + could not show value of type Tuple at array.jl:772
is ambiguous with: 
    + could not show value of type Tuple at range.jl:424.
To fix, define 
    + could not show value of type Tuple
before the new definition.
Warning: New definition 
    + could not show value of type Tuple at array.jl:772
is ambiguous with: 
    + could not show value of type Tuple at range.jl:425.
To fix, define 
    + could not show value of type Tuple
before the new definition.
Warning: New definition 
    - could not show value of type Tuple at array.jl:773
is ambiguous with: 
    - could not show value of type Tuple at range.jl:433.
To fix, define 
    - could not show value of type Tuple
before the new definition.
Warning: New definition 
    - could not show value of type Tuple at array.jl:773
is ambiguous with: 
    - could not show value of type Tuple at range.jl:434.
To fix, define 
    - could not show value of type Tuple
before the new definition.
Warning: New definition 
    - could not show value of type Tuple at array.jl:774
is ambiguous with: 
    - could not show value of type Tuple at range.jl:431.
To fix, define 
    - could not show value of type Tuple
before the new definition.


Warning: New definition 
    + could not show value of type Tuple at sparse/sparsematrix.jl:604
is ambiguous with: 
    + could not show value of type Tuple at array.jl:771.
To fix, define 
    + could not show value of type Tuple
before the new definition.
Warning: New definition 
    + could not show value of type Tuple at sparse/sparsematrix.jl:605
is ambiguous with: 
    + could not show value of type Tuple at array.jl:772.
To fix, define 
    + could not show value of type Tuple
before the new definition.
Warning: New definition 
    - could not show value of type Tuple at sparse/sparsematrix.jl:607
is ambiguous with: 
    - could not show value of type Tuple at array.jl:773.
To fix, define 
    - could not show value of type Tuple
before the new definition.
Warning: New definition 
    - could not show value of type Tuple at sparse/sparsematrix.jl:607
is ambiguous with: 
    - could not show value of type Tuple at array.jl:777.
To fix, define 
    - could not show value of type Tuple
before the new definition.

test_all passes though.

Besides the ambiguities, there's another problem: the warning reads like: could not show value of type Tuple ...

@lindahua
Copy link
Contributor Author

cc: @stevengj

It looks like it is due to the recent restoration of array +/- scalar

@JeffBezanson
Copy link
Sponsor Member

Ah; that's why he initially used StridedArray instead of AbstractArray :)

@stevengj
Copy link
Member

Should we switch #7226 back to StridedArray? Or resolve all the ambiguities?

(Yes, I've seen those "could not show value of type Tuple" ambiguity warnings before...they are annoying.)

@simonster
Copy link
Member

It looks like the ambiguity warnings arise because some AbstractArray types already implemented + and - involving scalars, which wasn't consistent with the previous decision either. (Here are the ambiguity warnings I get pasting the new definitions into my 1 day old Julia, which more clearly identify what is ambiguous.) Ideally I think we'd make .+/.- involving scalars the canonical operation that AbstractArray types should implement, which should just be a matter of removing/renaming the +/- methods. Then +/- involving scalars can be handled by the methods that @stevengj just added without ambiguity.

@lindahua
Copy link
Contributor Author

I agree with @simonster. Subtypes of AbstractArray (e.g. Range and SparseMatrixCSC) should specialize .+ and .-.

+ and - should only be defined on AbstractArrays, which simply delegate to .+ and .-. This should be documented somewhere, so package authors who want to make their own array types know what to do.

@stevengj
Copy link
Member

Sounds reasonable.

@simonster
Copy link
Member

One further issue is that operators.jl defines:

.+(x,y) = x+y
.-(x,y) = x-y

This can lead to a stack overflow when neither Array + Number nor Array .+ Number are defined, e.g. (0:1)+im

@stevengj
Copy link
Member

We should probably add new fallbacks for .+(AbstractArray, Number) etcetera

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

No branches or pull requests

4 participants