-
-
Notifications
You must be signed in to change notification settings - Fork 478
rand_distr: split gamma module #1464
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this change, I think one module per distribution is easier to keep a clear view over than when they are grouped together. I have a few thoughts/suggestions:
- Nearly all distributions define a type called
Errorwhich then gets re-exported with the name of the distribution prepended on, but these new modules define the error type name already with the distribution name in them. I would suggest staying consistent here. - Might want to give the same treatment to
ZetaandZipf StudentTmight deserve its own error type (alias), but this is definitely less important
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making those changes, looks good to me now!
Move Beta, Student's t, Fisher-F, Chi-squared and Zeta distributions to their own modules.
Move Beta, Student's t, Fisher-F and Chi-squared distributions to their own modules.
All recently added distributions (including the very minimal circle/disc/ball/sphere distributions) use their own module, and I find this makes PR reviews easier.