search

JuliaStats / GLM.jl

Generalized linear models in Julia
Other
593 stars 114 forks source link

More helpful errors #483

Open ParadaCarleton opened 2 years ago

ParadaCarleton commented 2 years ago 
julia> model = glm(@formula(y~x), data)
ERROR: MethodError: no method matching fit(::Type{GeneralizedLinearModel}, ::Matrix{Float64}, ::Vector{Float64})
Closest candidates are:
  fit(::Type{M}, ::AbstractMatrix{<:AbstractFloat}, ::AbstractVector{<:Real}, ::Distributions.UnivariateDistribution) where M<:GLM.AbstractGLM at ~/.julia/packages/GLM/DgddX/src/glmfit.jl:471
  fit(::Type{M}, ::AbstractMatrix{<:AbstractFloat}, ::AbstractVector{<:Real}, ::Distributions.UnivariateDistribution, ::Link; dofit, wts, offset, fitargs...) where M<:GLM.AbstractGLM at ~/.julia/packages/GLM/DgddX/src/glmfit.jl:471
  fit(::Type{M}, ::AbstractMatrix, ::AbstractVector, ::Distributions.UnivariateDistribution) where M<:GLM.AbstractGLM at ~/.julia/packages/GLM/DgddX/src/glmfit.jl:495

The error here was caused by forgetting to add a distribution. However, it might not be immediately clear to a user that the problem is a missing argument for glm (not fit). We could improve this by throwing an error saying glm requires a distribution.

model = glm(@formula(y~x), data, Normal)
ERROR: MethodError: no method matching fit(::Type{GeneralizedLinearModel}, ::Matrix{Float64}, ::Vector{Float64}, ::Type{Normal})
Closest candidates are:
  fit(::Type{StatsBase.Histogram}, ::Any...; kwargs...) at ~/.julia/packages/StatsBase/pJqvO/src/hist.jl:383
  fit(::Type{T}, ::FormulaTerm, ::Any, ::Any...; contrasts, kwargs...) where T<:RegressionModel at ~/.julia/packages/StatsModels/57Kc9/src/statsmodel.jl:78
  fit(::Type{T}, ::FormulaTerm, ::Any, ::Any...; contrasts, kwargs...) where T<:StatisticalModel at ~/.julia/packages/StatsModels/57Kc9/src/statsmodel.jl:78

Passing a type rather than a distribution caused the error here, but this is also not clear for a new user.

nalimilan commented 2 years ago

Yeah the signature of glm should be made stricter, to match that of fit. Would you make a PR?

Having a method that throws an error if a type is passed rather than an instance for the distribution could make sense. We could almost allow passing either...