mitchelloharawild / distributional

Vectorised distributions for R
https://pkg.mitchelloharawild.com/distributional
GNU General Public License v3.0
94 stars 15 forks source link

dist_wrap cannot search the user's environment for distributions #69

Closed mjskay closed 2 years ago

mjskay commented 2 years ago

I've run into a minor issue while trying to move some more internals of {ggdist} over to {distributional}.

{ggdist} allows users to specify distributions as character strings and then searches the calling environment for d/p/q/r functions to visualize the distribution. I tried to reimplement this on top of dist_wrap(), but hit a snag: dist_wrap() requires the distribution functions it wraps to be within a package, and this package to be specified when wrapping the distribution (using the package argument).

A minor issue with this is the (in my opinion) unexpected behavior that even if a package namespace is attached and you use dist_wrap() to wrap a distribution in that package, to you still have to specify the package name.

A bigger issue is that there is no way to use custom distributions unless they are defined in a package. So, for example, this does not work (because it defines the distribution functions in the user's environment):

library(gamlss.tr)

gen.trun(family = "NO")
#> A truncated family of distributions from NO has been generated 
#>  and saved under the names:  
#>  dNOtr pNOtr qNOtr rNOtr NOtr 
#> The type of truncation is left 
#>  and the truncation parameter is 0

dNOtr(0)
#> [1] 0.7978846

x = dist_wrap("NOtr")
#> Error in dist_wrap("NOtr"): could not find function "dist_wrap"
density(x, 0)
#> Error in density(x, 0): object 'x' not found

x = dist_wrap("NOtr", package = "gamlss.tr")
#> Error in dist_wrap("NOtr", package = "gamlss.tr"): could not find function "dist_wrap"
density(x, 0)
#> Error in density(x, 0): object 'x' not found

Created on 2021-10-27 by the reprex package (v2.0.1)

Could there be a way for dist_wrap() to instead search the calling environment for the distribution functions? E.g. the default value of package could be NULL instead of "stats", and when NULL the calling environment could be searched (which would basically always include {stats} anyway, and therefore not cause backwards compatibility issues). Or perhaps an environment instead of a package name could be passed, which would default to the calling environment of dist_wrap()?

If this sounds reasonable to you I'd be happy to file a PR. Thanks!

mitchelloharawild commented 2 years ago

Added, thanks for your suggestion! I don't think replicating references to environments here is a big overhead, but if it is then I'd have to work on #25 to fix it.

mjskay commented 2 years ago

Lovely, thanks so much!