Unhelpful/confusing error message when extending functions from Base

[ingmarschuster]

importing the module quoted below and calling
HeldoutDatasets.HoldoutIndex(10,0.2)

gave me the error

ERROR: no method getindex(Dict{Any,Any},Int64)
in HoldoutIndex at HeldoutDatasets.jl:23

which I find completely confusing and very hard to debug. Removing the module HeldoutDatasets declaration gave the following much clearer Error:

ERROR: error in method definition: function Base.getindex must be explicitly imported to be extended
in include_from_node1 at loading.jl:92
in reload_path at loading.jl:117
in reload at loading.jl:59
at HeldoutDatasets.jl:43

module HeldoutDatasets
using HoldOuts

type HoldoutIndex
    num_entities::Int64
    to_remove::Dict

    function HoldoutIndex(num_entities::Int64, hold_out = 0.2)
        max_idx =   floor(num_entities^2*hold_out)
        to_remove = Dict()
        for i in 1:max_idx
            r = rand(1:num_entities)
            c = r
            while c == r
                c = rand(1:num_entities)
            end
            if !haskey(to_remove, r)
                tmp = Array(Int64,0)
                push!(tmp, c)
                to_remove[r] = tmp
            else
                tmp = to_remove[convert(Any,r)]
                push!(tmp, c)
            end
        end
        for k in keys(to_remove)
            to_remove[k] = Reverse(sort(to_remove[k]))
        end
        return new(num_entities, to_remove)
    end
end

function getindex(d::HoldoutIndex, r::Number)
    retval = convert(Array{Int64,1},1:d.num_entities)
    if !haskey(d.to_remove, r)
        return retval
    end
    for c in d.to_remove[r]
        splice!(retval, c)
    end
    return retval
end

end

[ViralBShah]

Perhaps you need import Base.getindex?

[JeffBezanson]

The interesting issue here is that a[i] currently expands to a call to getindex in the current scope. Other magic syntax, like colon, a.b, and cat ([x,y]) directly call functions in Base. We should probably have them all call Base.

[JeffBezanson]

I should point out, with that change, the Dict lookup in HoldoutIndex would work, but the getindex definition still will not extend Base.getindex, so this issue would remain.

The trouble is that given function getindex we don't know whether the intent is to extend Base.getindex. For certain names (getindex among them) it seems pretty obvious, but it's hard to know where the line is. It's fairly common to define functions that conflict with Base and let modules do their thing; for example you might write function start() without realizing that Base.start exists. I don't want to give a warning for all such cases, since this would effectively reserve all names in Base.

[ingmarschuster]

Yes its hard to know where the line is, I agree. But you don't have to give a warning in general, just extend current error message

ERROR: no method getindex(Dict{Any,Any},Int64)
in HoldoutIndex at HeldoutDatasets.jl:23

to also include something like

getindex() in current scope shadows Base.getindex(Dict{Any,Any},Int64). Perhaps you forgot to explicitly import Base.getindex?

This way you just give a very helpful hint when the user is getting an error anyway, instead of a warning for every name clash with Base.

[JeffBezanson]

getindex is now in Base.Operators, which will avoid some of the confusion.

[JeffBezanson]

That was a good idea; I have implemented it.

[ingmarschuster]

wonderful, thanks.

[benjohnbarnes]

Hi,

I just happened on this thread while trying to extend rand. The new error message is helpful and it's cool to see the discussion that lead to it! :-) However, The error message I got was:

ERROR: error in method definition: function Random.rand must be explicitly imported to be extended

So, I tried "import Random", but got the error: ERROR: Random not found

Based on the discussion above, I tried: "import Base.Random"

Which doesn't give an error. Could the warning that you added be more specific and suggest the exact module to import, such as:

ERROR: error in method definition: functionrandinBase.Randommust be explicitly imported to be extended. Try "import Base.Random"

Or some such?

However, on the REPLY I still can't extend rand (same error. I tried the same in a file, and it does work (although it leads to a stack overflow because the method's calling itself, oops). So there's clearly still lots that I don't understand!

[timholy]

Does import Base.Random.rand work? The current error message is explicit in saying you need to import that specific function, but I agree with you adding Base. to the message would be helpful.

[benjohnbarnes]

On 28 Jan 2015, at 10:44, Tim Holy notifications@github.com wrote:

Does import Base.Random.rand work? The current error message is explicit in saying you need to import that specific function, but I agree with you adding Base. to the message would be helpful.

Hi Tim,

Thanks…

julia> rand(1:10)
3

julia> rand(thing) = thing[rand(1:length(thing))]
ERROR: error in method definition: function Random.rand must be explicitly imported to be extended

julia> import Random.rand
ERROR: Random not found
in require at loading.jl:47

julia> import Base.Random.rand

julia> rand(thing) = thing[rand(1:length(thing))]
rand (generic function with 33 methods)
julia> rand({1,2,3})
3

julia> rand({1,2,3})
3

julia> rand({1,2,3})
2

julia> rand({1,2,3})

Yeah – I think simply adding the explicit mention of "Base" would be very helpful.

I've also noticed that if start the REPL and my first line is:

rand(thing) = thing[rand(1:length(thing))]

Then I don't get the module error above, but I do get a stack overflow if I try to call my rand with an array.

However, if I call rand before I provide my declaration, my implementation works and doesn't lead to a stack overflow.

I'm speculating that in the REPL at least, when I call rand, it implicitly loads the module in some way and pulls in the various rand specialisations.

I then provide my implementation and it adds it as another specialisation. When I call rand, it works, because the other specialisations are available and there isn't a stack busting recursive call. However, if I provide the first definition of rand then for some reason it doesn't do the implicit module load and my rand's all that exists, so there is a recursive call and it goes boom?

Or is it something else?

For what it's worth, I found this fairly confusing (although, I hope I'm learning something!) – definitely user error by me, and clearly, I'm clueless and just starting out and out to read more guides, but could this be a less sharp edge?

Thanks anyway – I'm finding the language terrific!=