Distributions.jl
Distributions.jl copied to clipboard
JuliaStats
accept `Number` in `Exponential`
This is a start on #1684, with Exponential as an example to start the discussion, I'm not personally across the problems this may cause so this PR is a request for input and feedback on that.
The idea is to loosen the type of θ to be Number, and add a check for Complex numbers so those errors are avoided. More or different checks may be required.
I'm not totally satisfied with the combination of !(θ isa Complex) and isreal(θ) checks, but the idea is to make sure that even wrapped numbers are internally Real.
Codecov Report
Patch coverage is 66.66% of modified lines.
| Files Changed | Coverage |
|---|---|
| src/univariate/continuous/exponential.jl | 66.66% |
:loudspeaker: Thoughts on this report? Let us know!.
The problem here is related to Unitful, right? I'm happy to merge this if we need to, but first I'd like to check if it's possible to fix this in Unitful. @sostock ?
IMO this needs more thought as it affects much more than just Exponential (see https://github.com/JuliaStats/Distributions.jl/issues/1684). I also agree that the checks introduced in this PR are suboptimal.
Its not related to Unitful.jl - its a generic problem with Number.
See here: https://github.com/rafaqz/ModelParameters.jl/issues/51
I don't have bandwidth (or the required knowledge) to make this "optimal" or think through the implications, that is clearly a problem for devs of this package.
I was just proposing a solution that would work for other users of my own packages, as well as Unitful.jl
Another option would be to make the support for non-Real types opt-in, so that this package only supports Exponential{<:Real}, but other packages can add support for their types as package extensions.
For that, one would change the type parameter to T<:Number and rename the internal constructor to something like unsafe_exponential while providing an outer constructor only for Exponential{<:Real}. Other packages (like Unitful.jl) can then create a package extension in which they define their own outer constructor that calls the inner unsafe_exponential function.
