Configurations.jl icon indicating copy to clipboard operation
Configurations.jl copied to clipboard

Add more specific exception message when conversion fails

Open fatteneder opened this issue 3 years ago • 1 comments

Hi,

I might have a suggestions for an improvement of default error handling in terms of descriptiveness.

Example

@option struct MyType
   str::String = "abc"
end

dict_wrongkey = Dict("mystr"=>"abc")
from_dict(MyType, dict_wrongkey)

dict_wrongvaluetype = Dict("str"=>1)
from_dict(MyType, dict_wrongvaluetype)

The call from_dict(MyType, dict_wrongkey) will throw an InvalidKeyError with a descriptive message that mentions the invalid key,

ERROR: InvalidKeyError: invalid key mystr, possible keys are: str

whereas the latter gives a MethodError where one cannot see which parameter of the dictionary failed the conversion,

ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type String

Workaround

A workaround for this would be to overload the from_dict method generically, e.g.

function Configurations.from_dict(::Type{OptionType}, ::OptionField{f_name}, ::Type{T}, x::S) where {OptionType, f_name, T, S}
  try
    return convert(T, x)
  catch e
    if e isa MethodError && e.f == convert
      error("Failed to convert '$x' to type '$T' for field '$f_name'")
    end
  end
end

However, this feels a little hacky, because I think the exception handling should be separated from the conversion method.

Question: Would you be interested in reviewing a merge request that implements a new error type similar to InvalidKeyError?

Cheers

fatteneder avatar Jan 28 '22 15:01 fatteneder

Yes, I like this idea, I agree the error handling is less intuitive, please go ahead to implement a new error type

Roger-luo avatar Jan 28 '22 16:01 Roger-luo