Switch to keyword constructors

[simonbyrne]

Now that we have fast keywords (well, we will once JuliaLang/julia#29083 is fixed), I propose that we switch the constructors to use keywords. This has several advantages:

1. It would match what we currently print.
2. It makes clear what parameterisation is being used, e.g. Normal(μ=1.0, σ=2.0) makes clear that it is a standard deviation, not a variance.
3. With a bit of trickery, we can dispatch on different keywords to allow different constructors, e.g. we could support all the following:

Normal(μ=1.0, σ=2.0) # the default
Normal(mu=1.0, sigma=2.0) # ASCII fallback
Normal(mean=1.0, std=2.0) # moment mathing
Normal(mean=1.0, var=4.0) # alternative parameterisations

4. We could use the same thing for constrained fitting. e.g. fit(Normal, x, σ=2.0) could fit the MLE with σ constrained.

[simonbyrne]

(I suggested something like this in #584).

[matbesancon]

Looks nice, what do you mean by trickery for the dispatch?

[simonbyrne]

I've actually now put together a package for the trickery part:
https://github.com/simonbyrne/KeywordDispatch.jl

[matbesancon]

Not sure I'm convinced by using macros for all Distribution constructors. Unlike other packages, I appreciate this one can be read and used with just knowledge of plain Julia. This would mean people have to understand an other package to get how this one is built

…

On Wed, Sep 19, 2018, 20:54 Simon Byrne ***@***.***> wrote: I've actually now put together a package for the trickery part: https://github.com/simonbyrne/KeywordDispatch.jl — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#768 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHRRsoL_EOQ3mJ7U83kRDzgazJ8Hocq1ks5ucpL5gaJpZM4WfKTX> .

[kleinschmidt]

Could do the R trick and just accept all the keywords and error/warn and use one if both std and var are specified

[matbesancon]

About this kind of error, they could get a bit messy if yielded by a NoKwDispatchError or such

…

On Wed, Sep 19, 2018, 21:19 Dave Kleinschmidt ***@***.***> wrote: Could do the R trick and just accept all the keywords and error/warn and use one if both std and var are specified — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#768 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHRRsj_hwhnxCIxGX-zsPLWNjcAO7XT0ks5ucpi0gaJpZM4WfKTX> .

[simonbyrne]

Distributions already uses a lot of macros: we could put them in Distributions itself (which was my original plan), but it seemed easier to make it a separate package. I'm happy to make the errors more useful.

re: allowing all the keywords: I actually tied this a while ago, but I ended up with a lot of boilerplate if blocks, and the complexity increases exponentially as you add more arguments.

[kleinschmidt]

you could use coalesce or something to simplify a lot of it though right?

[matbesancon]

Valid points for the exponential kw checks for Distributions with lots of parameters, not sure how often this is the case

…

On Wed, Sep 19, 2018, 21:26 Simon Byrne ***@***.***> wrote: Distributions already uses a lot of macros: we could put them in Distributions itself (which was my original plan), but it seemed easier to make it a separate package. I'm happy to make the errors more useful. re: allowing all the keywords: I actually tied this a while ago, but I ended up with a lot of boilerplate if blocks, and the complexity increases exponentially as you add more arguments. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#768 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHRRsuPTKdvUioyZtGtl9_-joCsjfHw9ks5ucppJgaJpZM4WfKTX> .

[simonbyrne]

I've created a demonstration on the sb/keyword branch. An interesting example is the Gamma constructor:

Distributions.jl/src/univariate/continuous/gamma.jl

Lines 41 to 55 in 247ab6d

	@kwdispatch Gamma()
	@kwmethod Gamma(;α,θ) = Gamma(α,θ)
	@kwmethod Gamma(;alpha,theta) = Gamma(alpha,theta)
	@kwmethod Gamma(;shape,scale) = Gamma(shape,scale)
	@kwmethod Gamma(;α,β) = Gamma(α,1/β)
	@kwmethod Gamma(;alpha,beta) = Gamma(alpha,1/beta)
	@kwmethod Gamma(;shape,rate) = Gamma(shape,1/rate)
	@kwmethod function Gamma(;mean, var)
	θ=var/mean
	Gamma(mean/θ, θ)
	end
	@kwmethod Gamma(;mean, std) = Gamma(;mean=mean,var=std^2)

[matbesancon]

interesting just checked it. A good point is that we can use it only for some distributions and it's not a breaking change

[simonbyrne]

I would like to take the opportunity to deprecate some of the more confusing "default value" ones, e.g. Normal(x).

[taqtiqa-mark]

I think JuliaLang/julia#9498 may be relevant, if not internally, then for users. Not sure if it is ablocking issue or just one to be aware of. HTH?

[ghost]

Now that we have fast keywords (well, we will once JuliaLang/julia#29083 is fixed), I propose that we switch the constructors to use keywords.

Are there still any plans to add this to Distributions.jl? I've definitely felt like this could be very useful before.