Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small fixes & improvements #99

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Small fixes & improvements #99

merged 2 commits into from
Feb 2, 2024

Conversation

zeileis
Copy link
Collaborator

@zeileis zeileis commented Jan 23, 2024

I noticed that some quantile() and random() methods for certain distributions don't pass on the distribution vector as intended. This typically looked something like this:

quantile.dist <- function(x, probs, drop = TRUE, elementwise = NULL, ...) {
  FUN <- function(at, d) qdist(at, par = x$par, ...)
  apply_dpqr(d = x, FUN = FUN, at = probs, type = "quantile", drop = drop, elementwise = elementwise)
}

Note that x$par is passed on in FUN(at, d) rather than d$par. This never leads to any problems, though, because through lexical scoping the correct distribution vector is found nevertheless. But to employ standard scoping, I changed all of the occurrences to d$par for a number of distributions.

Finally, I made a small improvement in the ?Exponential documentation which addresses the "Lost braces" NOTE we get in the current CRAN checks.

@zeileis
Copy link
Collaborator Author

zeileis commented Jan 23, 2024

Re: Automatic checks. So far only one check flavor was successful and the others either failed or are still queued. The failures look like problems in the GitHub Action and not like problems in the actual check. So just fyi: I ran checks locally on my machine with R-release and R-devel, both of which were successful.

@alexpghayes
Copy link
Owner

alexpghayes commented Jan 24, 2024 via email

@zeileis
Copy link
Collaborator Author

zeileis commented Jan 24, 2024

No worries, thanks for the quick feedback!

P.S.: In the next weeks, I'll also try to have a go at some alternative parameterizations for certain distributions (discussed in #7 and #20). We already have that for the NegativeBinomial but I'll try to add Beta, Exponential, Gamma, and Normal if I get round to it.

@alexpghayes alexpghayes merged commit 794ce48 into main Feb 2, 2024
1 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants