-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Print a tip in MethodError #57339
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
base: master
Are you sure you want to change the base?
Print a tip in MethodError #57339
Conversation
base/errorshow.jl
Outdated
# if the function has only one method, print a tip about the number of arguments to be used | ||
if f isa Function && length(methods(f)) == 1 | ||
print(io, "\n\nTip: the function `$f` was called with ", length(arg_types_param), " arguments, but has only one method accepting ", length(methods(f)[1].sig.parameters)-1, " argument.\n") |
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.
I'm not sure this is quite right; f
doesn't need to be a function in order for there to be a mismatch in the number of arguments (i.e. it could be a type or callable object), but also, I believe this tip will be printed when the correct number of arguments is provided but the sole method does not apply to them. For example, consider:
julia> f(x::Int) = x + 1
f (generic function with 1 method)
julia> f("hi")
ERROR: MethodError: no method matching f(::String)
The function `f` exists, but no method is defined for this combination of argument types.
Closest candidates are:
f(::Int64)
@ Main REPL[1]:1
Stacktrace:
[1] top-level scope
@ REPL[2]:1
As currently implemented, the error message would include
Tip: the function `f` was called with 1 arguments, but has only one method accepting 1 argument.
which I don't think would help a user determine the source of the issue or how to fix it.
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.
That is correct I didnt think about this. But could be easly resolved by also checking if the number of arguments with wich the function was called is different from the accepted arguments, like so:
if f isa Function && length(methods(f)) == 1 && length(arg_types_param) !== length(methods(f)[1].sig.parameters)-1
print(...)
end
can you think of other cases in wich this tip could be misleading?
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.
That's a good start but I don't think it will handle vararg signatures correctly. For example, consider:
julia> f(x::Int, y::Float64...) = x + sum(y; init=0.0)
f (generic function with 1 method)
julia> m = only(methods(f));
julia> nargs = length(m.sig.parameters) - 1 # number of defined arguments in the signature
2
julia> f(1) # provided arguments < nargs, no error
1.0
julia> f(1.0) # provided arguments < nargs but error is irrelevant to that
ERROR: MethodError: no method matching f(::Float64)
[...]
julia> f(1, 2) # provided arguments == nargs
ERROR: MethodError: no method matching f(::Int64, ::Int64)
[...]
julia> f(1, 2, 3) # provided arguments > nargs
ERROR: MethodError: no method matching f(::Int64, ::Int64, ::Int64)
[...]
You could use isvatuple(m.sig)
to identify this case.
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 that's true I didn't tought about those, in fact when the function method accepts vararg types it doesn't make sense to talk about the number of arguments passed. I also realized that a function with keyword arguments would wrongly trigger the tip:
julia> h(x::Int64; kwdarg::Int64=1) = x+kwdard
h (generic function with 1 method)
julia> h(1.0,1)
...
Tip: the function `h` was called with 2 arguments, but has only one method accepting 1 arguments.
...
bc in this case the error is caused by the first element being a float, not by the number of types.
So to wrap up this tip can be useful only when:
- the function has only one method
- the n of arguments provided is differrent from the n of arguments accepted by the method
- the arguments accepted are not of type vararg
- the function does not accept keyword arguments
can you think of other cases in wich this tip could be misleading?
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.
It could actually also be helpful if the function has multiple methods, all of which take the same (fixed) number of arguments. Then one could say "You passed 1 argument but all methods of this function expect 2 arguments"
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.
- the n of arguments provided is differrent from the n of arguments accepted by the method
- the arguments accepted are not of type vararg
I think there is still a useful query here, even if you have vararg. For example, using the definition f(x::Int, y::Float64...)
from above, the error from calling f()
would be relevant to the number of arguments provided.
I think something like this would cover the cases where this tip could provide some useful information:
ms = methods(f)
n = #= number of arguments provided to call =#
if !isempty(ms)
toomany = all(m -> !isvatuple(m.sig) && n > m.nargs - 1, ms)
toofew = all(m -> n < m.nargs - 1 - Int(isvatuple(m.sig)), ms)
if toofew || toomany
print(io, "No methods of `", f, "` accept ", n, " arguments; ",
toofew ? "fewer" : "more", "arguments provided than expected ",
"by any defined method.")
end
end
This seems relevant even in the case of h
as defined above that accepts keyword arguments.
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 seems a robuste way of handling all the cases. I updated the pr with something similar, that also prints a different tip in the case mentioned by @fingolfin in which all the methods have the exact same number of arguments. This because this "Hint" is tought for the less experienced julia developers, in the original issue is was related to the case:
accumulate([1,2,3,4,5]; init=0) do (old, new)
old + new
end
Hint: no method of `#5` accepts 2 arguments, all methods accept 1 arguments.
In this case for less experienced developers would help more to see written down that all methods accept one argument, rather than "more arguments provided than expected by any defined method".
I added tests with the main cases we talked about in the conversation.
ms = methods(f) | ||
if !isempty(ms) | ||
n = length(arg_types_param) # number of arguments provided by the user | ||
samenum = all(m -> m.nargs == ms[1].nargs, methods(f)) |
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 is wrong in the presence of varargs. For example for f
:
f(x, y) = 0
f(x, y...) = 0
it will print
all methods accept 2 arguments.
samenum = all(m -> m.nargs == ms[1].nargs, methods(f)) | ||
toofew = all(m -> n < m.nargs - 1 - Int(isvatuple(m.sig)), ms) | ||
toomany = all(m -> !isvatuple(m.sig) && n > m.nargs - 1, ms) | ||
if toofew || toomany | ||
print(io, | ||
"\nHint: no method of `", f, "` accepts ", n, " arguments, ", | ||
samenum ? "all methods accept " * string(ms[1].nargs - 1) * " arguments.\n" : | ||
(toofew ? "fewer" : "more") * " arguments provided than expected by any defined method.\n", | ||
) | ||
|
||
end |
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.
samenum = all(m -> m.nargs == ms[1].nargs, methods(f)) | |
toofew = all(m -> n < m.nargs - 1 - Int(isvatuple(m.sig)), ms) | |
toomany = all(m -> !isvatuple(m.sig) && n > m.nargs - 1, ms) | |
if toofew || toomany | |
print(io, | |
"\nHint: no method of `", f, "` accepts ", n, " arguments, ", | |
samenum ? "all methods accept " * string(ms[1].nargs - 1) * " arguments.\n" : | |
(toofew ? "fewer" : "more") * " arguments provided than expected by any defined method.\n", | |
) | |
end | |
lo, hi = extrema(m -> m.nargs - isvatuple(m.sig) - 1, ms) | |
if any(m -> isvatuple(m.sig), ms) | |
hi = typemax(hi) | |
end | |
if n < lo || hi < n | |
print(io, "\nHint: no method of `", f, "` accepts ", n, " arguments, all methods accept ",) | |
if lo == hi | |
println(io, "exactly ", lo, " argument", lo == 1 ? "" : "s", ".") | |
elseif hi == typemax(hi) | |
println(io, lo, " or more arguments.") | |
else | |
println(io, lo, " to ", hi, " arguments.") | |
end | |
end |
IMO this is more clear, both the code and the hints.
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.
Note that the Base.
qualification for isvatuple
is unnecessary since this code is inside of Base so isvatuple
is in scope
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.
edited
If a function with only one method accepting a certain number of arguments, gets called with a different number of arguments, a Tip is printed in the MethodError like this:
See the original pull request and issue #56325