Fix method ambiguities & unbound parameters + add Aqua tests (#1804)

* Fix method ambiguities & unbound parameters + add Aqua tests

* Fix compat entries for Julia 1.3

* Update aqua.jl

Project.toml

@@ -30,15 +30,26 @@ DistributionsDensityInterfaceExt = "DensityInterface"
 DistributionsTestExt = "Test"
 
 [compat]
+Aqua = "0.8"
+Calculus = "0.5"
 ChainRulesCore = "1"
+ChainRulesTestUtils = "1"
 DensityInterface = "0.4"
+Distributed = "<0.0.1, 1"
 FillArrays = "0.9, 0.10, 0.11, 0.12, 0.13, 1"
+FiniteDifferences = "0.12"
+ForwardDiff = "0.10"
+JSON = "0.21"
 LinearAlgebra = "<0.0.1, 1"
+OffsetArrays = "1"
 PDMats = "0.10, 0.11"
 Printf = "<0.0.1, 1"
 QuadGK = "2"
 Random = "<0.0.1, 1"
+SparseArrays = "<0.0.1, 1"
 SpecialFunctions = "1.2, 2"
+StableRNGs = "1"
+StaticArrays = "1"
 Statistics = "1"
 StatsAPI = "1.6"
 StatsBase = "0.32, 0.33, 0.34"
@@ -47,6 +58,7 @@ Test = "<0.0.1, 1"
 julia = "1.3"
 
 [extras]
+Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 Calculus = "49dc2e85-a5d0-5ad3-a950-438e2897f1b9"
 ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
 ChainRulesTestUtils = "cdddcdb0-9152-4a09-a978-84456f9df70a"
@@ -62,4 +74,4 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["StableRNGs", "Calculus", "ChainRulesCore", "ChainRulesTestUtils", "DensityInterface", "Distributed", "FiniteDifferences", "ForwardDiff", "JSON", "SparseArrays", "StaticArrays", "Test", "OffsetArrays"]
+test = ["Aqua", "StableRNGs", "Calculus", "ChainRulesCore", "ChainRulesTestUtils", "DensityInterface", "Distributed", "FiniteDifferences", "ForwardDiff", "JSON", "SparseArrays", "StaticArrays", "Test", "OffsetArrays"]

README.md

@@ -4,6 +4,7 @@ Distributions.jl
 [![Build Status](https://github.com/JuliaStats/Distributions.jl/workflows/CI/badge.svg)](https://github.com/JuliaStats/Distributions.jl/actions)
 [![](https://zenodo.org/badge/DOI/10.5281/zenodo.2647458.svg)](https://zenodo.org/record/2647458)
 [![Coverage Status](https://coveralls.io/repos/JuliaStats/Distributions.jl/badge.svg?branch=master)](https://coveralls.io/r/JuliaStats/Distributions.jl?branch=master)
+[![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)
 
 [![](https://img.shields.io/badge/docs-latest-blue.svg)](https://JuliaStats.github.io/Distributions.jl/latest/)
 [![](https://img.shields.io/badge/docs-stable-blue.svg)](https://JuliaStats.github.io/Distributions.jl/stable/)

src/censored.jl

@@ -431,6 +431,7 @@ _in_open_interval(x::Real, l::Real, ::Nothing) = x > l
 _clamp(x, l, u) = clamp(x, l, u)
 _clamp(x, ::Nothing, u) = min(x, u)
 _clamp(x, l, ::Nothing) = max(x, l)
+_clamp(x, ::Nothing, u::Nothing) = x
 
 _to_truncated(d::Censored) = truncated(d.uncensored, d.lower, d.upper)
 

src/common.jl

@@ -212,13 +212,25 @@ usually it is sufficient to implement `logpdf`.
 See also: [`logpdf`](@ref).
 """
 @inline function pdf(
-    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N}
-) where {N}
-    @boundscheck begin
-        size(x) == size(d) ||
-            throw(DimensionMismatch("inconsistent array dimensions"))
+    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M}
+) where {N,M}
+    if M == N
+        @boundscheck begin
+            size(x) == size(d) ||
+                throw(DimensionMismatch("inconsistent array dimensions"))
+        end
+        return _pdf(d, x)
+    else
+        @boundscheck begin
+            M > N ||
+                throw(DimensionMismatch(
+                    "number of dimensions of the variates ($M) must be greater than or equal to the dimension of the distribution ($N)"
+                ))
+            ntuple(i -> size(x, i), Val(N)) == size(d) ||
+                throw(DimensionMismatch("inconsistent array dimensions"))
+        end
+        return @inbounds map(Base.Fix1(pdf, d), eachvariate(x, variate_form(typeof(d))))
     end
-    return _pdf(d, x)
 end
 
 function _pdf(d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N}) where {N}
@@ -241,13 +253,25 @@ size of `x`.
 See also: [`pdf`](@ref).
 """
 @inline function logpdf(
-    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N}
-) where {N}
-    @boundscheck begin
-        size(x) == size(d) ||
-            throw(DimensionMismatch("inconsistent array dimensions"))
+    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M}
+) where {N,M}
+    if M == N
+        @boundscheck begin
+            size(x) == size(d) ||
+                throw(DimensionMismatch("inconsistent array dimensions"))
+        end
+        return _logpdf(d, x)
+    else
+        @boundscheck begin
+            M > N ||
+                throw(DimensionMismatch(
+                    "number of dimensions of the variates ($M) must be greater than or equal to the dimension of the distribution ($N)"
+                ))
+            ntuple(i -> size(x, i), Val(N)) == size(d) ||
+                throw(DimensionMismatch("inconsistent array dimensions"))
+        end
+        return @inbounds map(Base.Fix1(logpdf, d), eachvariate(x, variate_form(typeof(d))))
     end
-    return _logpdf(d, x)
 end
 
 # `_logpdf` should be implemented and has no default definition
@@ -272,20 +296,6 @@ Base.@propagate_inbounds function pdf(
     return map(Base.Fix1(pdf, d), x)
 end
 
-@inline function pdf(
-    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M},
-) where {N,M}
-    @boundscheck begin
-        M > N ||
-            throw(DimensionMismatch(
-                "number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
-            ))
-        ntuple(i -> size(x, i), Val(N)) == size(d) ||
-            throw(DimensionMismatch("inconsistent array dimensions"))
-    end
-    return @inbounds map(Base.Fix1(pdf, d), eachvariate(x, variate_form(typeof(d))))
-end
-
 """
     logpdf(d::Distribution{ArrayLikeVariate{N}}, x) where {N}
 
@@ -305,20 +315,6 @@ Base.@propagate_inbounds function logpdf(
     return map(Base.Fix1(logpdf, d), x)
 end
 
-@inline function logpdf(
-    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M},
-) where {N,M}
-    @boundscheck begin
-        M > N ||
-            throw(DimensionMismatch(
-                "number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
-            ))
-        ntuple(i -> size(x, i), Val(N)) == size(d) ||
-            throw(DimensionMismatch("inconsistent array dimensions"))
-    end
-    return @inbounds map(Base.Fix1(logpdf, d), eachvariate(x, variate_form(typeof(d))))
-end
-
 """
     pdf!(out, d::Distribution{ArrayLikeVariate{N}}, x) where {N}
 
@@ -365,7 +361,7 @@ end
     @boundscheck begin
         M > N ||
             throw(DimensionMismatch(
-                "number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
+                "number of dimensions of the variates ($M) must be greater than the dimension of the distribution ($N)"
             ))
         ntuple(i -> size(x, i), Val(N)) == size(d) ||
             throw(DimensionMismatch("inconsistent array dimensions"))
@@ -414,7 +410,7 @@ See also: [`pdf!`](@ref).
     @boundscheck begin
         M > N ||
             throw(DimensionMismatch(
-                "number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
+                "number of dimensions of the variates ($M) must be greater than the dimension of the distribution ($N)"
             ))
         ntuple(i -> size(x, i), Val(N)) == size(d) ||
             throw(DimensionMismatch("inconsistent array dimensions"))
@@ -445,23 +441,22 @@ be
 - an array of dimension `N + 1` with `size(x)[1:N] == size(d)`, or
 - an array of arrays `xi` of dimension `N` with `size(xi) == size(d)`.
 """
-Base.@propagate_inbounds function loglikelihood(
-    d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N},
-) where {N}
-    return logpdf(d, x)
-end
-@inline function loglikelihood(
+Base.@propagate_inbounds @inline function loglikelihood(
     d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M},
 ) where {N,M}
-    @boundscheck begin
-        M > N ||
-            throw(DimensionMismatch(
-                "number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
-            ))
-        ntuple(i -> size(x, i), Val(N)) == size(d) ||
-            throw(DimensionMismatch("inconsistent array dimensions"))
+    if M == N
+        return logpdf(d, x)
+    else
+        @boundscheck begin
+            M > N ||
+                throw(DimensionMismatch(
+                    "number of dimensions of the variates ($M) must be greater than or equal to the dimension of the distribution ($N)"
+                ))
+            ntuple(i -> size(x, i), Val(N)) == size(d) ||
+                throw(DimensionMismatch("inconsistent array dimensions"))
+        end
+        return @inbounds sum(Base.Fix1(logpdf, d), eachvariate(x, ArrayLikeVariate{N}))
     end
-    return @inbounds sum(Base.Fix1(logpdf, d), eachvariate(x, ArrayLikeVariate{N}))
 end
 Base.@propagate_inbounds function loglikelihood(
     d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:AbstractArray{<:Real,N}},

src/deprecates.jl

@@ -34,9 +34,9 @@ for fun in [:pdf, :logpdf,
     fun! = Symbol(fun, '!')
 
     @eval begin
-        @deprecate ($_fun!)(r::AbstractArray, d::UnivariateDistribution, X::AbstractArray) r .= ($fun).(d, X) false
-        @deprecate ($fun!)(r::AbstractArray, d::UnivariateDistribution, X::AbstractArray) r .= ($fun).(d, X) false
-        @deprecate ($fun)(d::UnivariateDistribution, X::AbstractArray) ($fun).(d, X)
+        @deprecate ($_fun!)(r::AbstractArray{<:Real}, d::UnivariateDistribution, X::AbstractArray{<:Real}) r .= ($fun).(d, X) false
+        @deprecate ($fun!)(r::AbstractArray{<:Real}, d::UnivariateDistribution, X::AbstractArray{<:Real}) r .= ($fun).(d, X) false
+        @deprecate ($fun)(d::UnivariateDistribution, X::AbstractArray{<:Real}) ($fun).(d, X)
     end
 end
 

src/mixtures/mixturemodel.jl

@@ -362,14 +362,14 @@ end
 pdf(d::UnivariateMixture, x::Real) = _mixpdf1(d, x)
 logpdf(d::UnivariateMixture, x::Real) = _mixlogpdf1(d, x)
 
-_pdf!(r::AbstractArray, d::UnivariateMixture{Discrete}, x::UnitRange) = _mixpdf!(r, d, x)
-_pdf!(r::AbstractArray, d::UnivariateMixture, x::AbstractArray) = _mixpdf!(r, d, x)
-_logpdf!(r::AbstractArray, d::UnivariateMixture, x::AbstractArray) = _mixlogpdf!(r, d, x)
-
-_pdf(d::MultivariateMixture, x::AbstractVector) = _mixpdf1(d, x)
-_logpdf(d::MultivariateMixture, x::AbstractVector) = _mixlogpdf1(d, x)
-_pdf!(r::AbstractArray, d::MultivariateMixture, x::AbstractMatrix) = _mixpdf!(r, d, x)
-_logpdf!(r::AbstractArray, d::MultivariateMixture, x::AbstractMatrix) = _mixlogpdf!(r, d, x)
+_pdf!(r::AbstractArray{<:Real}, d::UnivariateMixture{Discrete}, x::UnitRange) = _mixpdf!(r, d, x)
+_pdf!(r::AbstractArray{<:Real}, d::UnivariateMixture, x::AbstractArray{<:Real}) = _mixpdf!(r, d, x)
+_logpdf!(r::AbstractArray{<:Real}, d::UnivariateMixture, x::AbstractArray{<:Real}) = _mixlogpdf!(r, d, x)
+
+_pdf(d::MultivariateMixture, x::AbstractVector{<:Real}) = _mixpdf1(d, x)
+_logpdf(d::MultivariateMixture, x::AbstractVector{<:Real}) = _mixlogpdf1(d, x)
+_pdf!(r::AbstractArray{<:Real}, d::MultivariateMixture, x::AbstractMatrix{<:Real}) = _mixpdf!(r, d, x)
+_logpdf!(r::AbstractArray{<:Real}, d::MultivariateMixture, x::AbstractMatrix{<:Real}) = _mixlogpdf!(r, d, x)
 
 
 ## component-wise pdf and logpdf

src/multivariate/mvtdist.jl

@@ -139,7 +139,7 @@ function _logpdf(d::AbstractMvTDist, x::AbstractVector{T}) where T<:Real
     v - shdfhdim * log1p(sqmahal(d, x) / d.df)
 end
 
-function _logpdf!(r::AbstractArray, d::AbstractMvTDist, x::AbstractMatrix)
+function _logpdf!(r::AbstractArray{<:Real}, d::AbstractMvTDist, x::AbstractMatrix{<:Real})
     sqmahal!(r, d, x)
     shdfhdim, v = mvtdist_consts(d)
     for i = 1:size(x, 2)

src/product.jl

@@ -12,19 +12,21 @@ struct ProductDistribution{N,M,D,S<:ValueSupport,T} <: Distribution{ArrayLikeVar
     size::Dims{N}
 
     function ProductDistribution{N,M,D}(dists::D) where {N,M,D}
-        isempty(dists) && error("product distribution must consist of at least one distribution")
+        if isempty(dists)
+            throw(ArgumentError("a product distribution must consist of at least one distribution"))
+        end
         return new{N,M,D,_product_valuesupport(dists),_product_eltype(dists)}(
             dists,
             _product_size(dists),
         )
     end
 end
 
-function ProductDistribution(dists::AbstractArray{<:Distribution{ArrayLikeVariate{M}},N}) where {M,N}
+function ProductDistribution(dists::AbstractArray{<:Distribution{<:ArrayLikeVariate{M}},N}) where {M,N}
     return ProductDistribution{M + N,M,typeof(dists)}(dists)
 end
 
-function ProductDistribution(dists::NTuple{N,Distribution{ArrayLikeVariate{M}}}) where {M,N}
+function ProductDistribution(dists::Tuple{Distribution{<:ArrayLikeVariate{M}},Vararg{Distribution{<:ArrayLikeVariate{M}}}}) where {M}
     return ProductDistribution{M + 1,M,typeof(dists)}(dists)
 end
 
@@ -54,10 +56,10 @@ function _product_size(dists::AbstractArray{<:Distribution{<:ArrayLikeVariate{M}
     size_dists = size(dists)
     return ntuple(i -> i <= M ? size_d[i] : size_dists[i-M], Val(M + N))
 end
-function _product_size(dists::NTuple{N,Distribution{<:ArrayLikeVariate{M}}}) where {M,N}
+function _product_size(dists::Tuple{Distribution{<:ArrayLikeVariate{M}},Vararg{Distribution{<:ArrayLikeVariate{M}}, N}}) where {M,N}
     size_d = size(first(dists))
     all(size(d) == size_d for d in dists) || error("all distributions must be of the same size")
-    return ntuple(i -> i <= M ? size_d[i] : N, Val(M + 1))
+    return ntuple(i -> i <= M ? size_d[i] : N + 1, Val(M + 1))
 end
 
 ## aliases
@@ -167,7 +169,7 @@ function _rand!(
 end
 
 # `_logpdf` for arrays of distributions
-# we have to fix a method ambiguity
+# we have to fix some method ambiguities
 _logpdf(d::ProductDistribution{N}, x::AbstractArray{<:Real,N}) where {N} = __logpdf(d, x)
 _logpdf(d::ProductDistribution{2}, x::AbstractMatrix{<:Real}) = __logpdf(d, x)
 function __logpdf(

src/univariate/discrete/categorical.jl

@@ -89,7 +89,7 @@ function pdf(d::Categorical, x::Real)
     return insupport(d, x) ? ps[round(Int, x)] : zero(eltype(ps))
 end
 
-function _pdf!(r::AbstractArray, d::Categorical{T}, rgn::UnitRange) where {T<:Real}
+function _pdf!(r::AbstractArray{<:Real}, d::Categorical{T}, rgn::UnitRange) where {T<:Real}
     vfirst = round(Int, first(rgn))
     vlast = round(Int, last(rgn))
     vl = max(vfirst, 1)

src/univariates.jl

@@ -311,7 +311,7 @@ See also: [`logpdf`](@ref).
 pdf(d::UnivariateDistribution, x::Real) = exp(logpdf(d, x))
 
 # extract value from array of zero dimension
-_pdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = pdf(d, first(x))
+pdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = pdf(d, first(x))
 
 """
     logpdf(d::UnivariateDistribution, x::Real)
@@ -323,7 +323,7 @@ See also: [`pdf`](@ref).
 logpdf(d::UnivariateDistribution, x::Real)
 
 # extract value from array of zero dimension
-_logpdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = logpdf(d, first(x))
+logpdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = logpdf(d, first(x))
 
 # loglikelihood for `Real`
 Base.@propagate_inbounds loglikelihood(d::UnivariateDistribution, x::Real) = logpdf(d, x)
@@ -452,7 +452,7 @@ function _pdf_fill_outside!(r::AbstractArray, d::DiscreteUnivariateDistribution,
     return vl, vr, vfirst, vlast
 end
 
-function _pdf!(r::AbstractArray, d::DiscreteUnivariateDistribution, X::UnitRange)
+function _pdf!(r::AbstractArray{<:Real}, d::DiscreteUnivariateDistribution, X::UnitRange)
     vl,vr, vfirst, vlast = _pdf_fill_outside!(r, d, X)
 
     # fill central part: with non-zero pdf

test/aqua.jl

@@ -0,0 +1,21 @@
+using Distributions
+using Test
+
+import Aqua
+
+@testset "Aqua" begin
+    # Test ambiguities separately without Base and Core
+    # Ref: https://github.com/JuliaTesting/Aqua.jl/issues/77
+    Aqua.test_all(
+        Distributions;
+        ambiguities = false,
+        # On older Julia versions, installed dependencies are quite old
+        # Thus unbound type parameters show up that are fixed in newer versions
+        unbound_args = VERSION >= v"1.6",
+    )
+    # Tests are not reliable on older Julia versions and
+    # show ambiguities in loaded packages
+    if VERSION >= v"1.6"
+        Aqua.test_ambiguities(Distributions)
+    end
+end

test/runtests.jl

@@ -11,6 +11,7 @@ import JSON
 import ForwardDiff
 
 const tests = [
+    "aqua",
     "univariate/continuous/loguniform",
     "univariate/continuous/arcsine",
     "univariate/discrete/dirac",
@@ -174,8 +175,3 @@ include("testutils.jl")
         include("$t.jl")
     end
 end
-
-# print method ambiguities
-println("Potentially stale exports: ")
-display(Test.detect_ambiguities(Distributions))
-println()