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

Unify convert errors #51655

Open
LilithHafner opened this issue Oct 10, 2023 · 2 comments
Open

Unify convert errors #51655

LilithHafner opened this issue Oct 10, 2023 · 2 comments
Labels
error messages Better, more actionable error messages

Comments

@LilithHafner
Copy link
Member

I noticed that Missings.jl started failing on nightly after, I think, #49470. The failure is due to Missings.jl depending on the error type of a certain failure, so technically this breakage is their fault. However, looking into how this came to be, I found that convert throws some really inconsistent errors:

julia> convert(Missing, 1)
ERROR: cannot convert a value to missing for assignment
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] nonmissingtype_checked(T::Type)
   @ Base ./missing.jl:44
 [3] convert(::Type{Missing}, x::Int64)
   @ Base ./missing.jl:70
 [4] top-level scope
   @ REPL[36]:1

julia> convert(1, Missing)
ERROR: MethodError: First argument to `convert` must be a Type, got 1
Stacktrace:
 [1] top-level scope
   @ REPL[37]:1

julia> convert(Int, missing)
ERROR: MethodError: Cannot `convert` an object of type Missing to an object of type Int64

Closest candidates are:
  convert(::Type{T}, ::T) where T<:Number
   @ Base number.jl:6
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:84
  convert(::Type{T}, ::Number) where T<:Number
   @ Base number.jl:7
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[38]:1

julia> convert(Union{}, missing)
ERROR: ArgumentError: cannot convert a value to Union{} for assignment # This is new, it used to be a MethodError
Stacktrace:
 [1] convert(T::Core.TypeofBottom, x::Missing)
   @ Base ./essentials.jl:323
 [2] top-level scope
   @ REPL[39]:1

It would be nice for them to all be consistent—the same error type at least.

@LilithHafner LilithHafner added the error messages Better, more actionable error messages label Oct 10, 2023
@vtjnash
Copy link
Member

vtjnash commented Oct 10, 2023

Error types, unless explicitly documented (like DomainError or Base.IOError), are subject to change is the official policy

@LilithHafner
Copy link
Member Author

That's why I said "technically this breakage is their fault." I still think there is room for improvement in convert error messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Better, more actionable error messages
Projects
None yet
Development

No branches or pull requests

2 participants