-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
DOC: Add example for np.random.default_rng().beta()
#25349
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
This reverts commit bc2b152.
Thank you for the contribution! I'd like to see it fleshed out a little more. See my comments on an earlier PR for the kinds of things that we'd like to see in examples for these distribution methods. Showing just one call of the method with no commentary is below the threshold (at least in my opinion). |
@rkern I hope the improved example is to your liking. Is there some guide/heuristic on how well document specific functions should be? |
A good general rule is to look at other functions/methods that are similar, and see what their documentation looks like. Fortunately, for these distribution methods on Specifically for these methods, I find that plotting the histogram of a large sample is typically more informative than showing the raw values of a handful of samples. Plotting the histogram (potentially with the analytical PDF) is probably the first priority (we have more methods where that's the only thing in the As you look at the other methods' example, please note that you will see things like If you do show the raw values in the outputs, or other computations based on the random output (e.g. computing means), please add the |
np.random.default_rng().beta()
np.random.default_rng().beta()
Co-authored-by: Robert Kern <robert.kern@gmail.com>
Thank you for taking so much time to explain everything clearly. Is it ok to use I also added the comparison to the order statistics but it might take up too much space. We could refactor the pdf into its own function to save space. |
I am not sure we should have too many examples in the function docstring. A couple of justifications:
I would be happier if we refer users to a wikipedia page or open a new section in the user guide around the theory and practice of the random module. We could then consider CI management to not build the user guide on every PR. |
@mattip ok. Would you prefer that I put in something like
because right now there is no example at all or should I just close this? |
@rkern: am I being too harsh? Was the ask to add examples to |
Per #21351 from the Docs Team:
Given that, I'd say that overcomes the minor objections concerning things like the wheel size. I do think that all of the distributions benefit from at least one plot of a typical distribution, to trigger visual memory. If it can be demonstrated that if we expanded the example for all of the distribution methods to include multiple plots, then the making of the plots and saving them balloon the CI times by noticeable amounts, then I'd be happy to have a soft-limit guideline of keeping things down to 1 plot per method. I also think that most of the methods could use a simple demonstration of what each of the parameters mean. Particularly in cases like this where we have two that that are hard to distinguish. I think it's really good to have that final example that shows when While each distribution method should link to the appropriate Wikipedia article, and we shouldn't recapitulate every interesting fact about the distribution, we should have our own quick demonstrations of the effects of the method arguments. We often have nonstandard parameterizations that confuse people who use the Wikipedia article as a reference. I do think this PR is a little on the high end of what we want (and thank you @linus-md for being patient with us while we whip you round-and-around on this; we all currently have different ideas of what we want). If I'd drop something, it would be the demonstration of the relationship to the explicit order statistics of the uniforms; it's a nice-to-have example, but if we're feeling pressure to slim this down, that would be the one; it's more about the distribution itself than using the method. But overall, I think this is in range. |
Relates to #21351.