Opened 4 years ago
Closed 4 years ago
#17936 closed defect (fixed)
Documentation of QQ.random_element
Reported by: | gagern | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.6 |
Component: | documentation | Keywords: | |
Cc: | Merged in: | ||
Authors: | Martin von Gagern | Reviewers: | Marc Mezzarobba |
Report Upstream: | N/A | Work issues: | |
Branch: | 624ca17 (Commits) | Commit: | 624ca17304d13677a11c93b9f8fd05c77976bdb7 |
Dependencies: | Stopgaps: |
Description
I find the documentation of QQ.random_element
to be lacking.
The arguments num_bound
and denom_bound
are not described. There is no INPUT section at all. The examples use these arguments, but without explaining their meaning either.
There is a note that extra arguments are “passed through”, but there is no saying where they get passed (namely to ZZ.random_element
). One really needs to look at the source code in order to understand this method.
Change History (4)
comment:1 Changed 4 years ago by
- Branch set to u/gagern/ticket/17936
comment:2 Changed 4 years ago by
- Commit set to 624ca17304d13677a11c93b9f8fd05c77976bdb7
- Status changed from new to needs_review
comment:3 Changed 4 years ago by
- Reviewers set to Marc Mezzarobba
- Status changed from needs_review to positive_review
Thanks!
comment:4 Changed 4 years ago by
- Branch changed from u/gagern/ticket/17936 to 624ca17304d13677a11c93b9f8fd05c77976bdb7
- Resolution set to fixed
- Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
Here is a commit improving the documentation. I've also dropped the bounds from the example with the 1/n distribution, since that distribution ignores all bounds. And I've repeated the examples a couple of times, since at my first try I got too many strange numbers which don't demonstrate the method, like integers and zeros. An alternative would be increasing the bounds, but at least for the simple no-args example that's no solution.
New commits:
Improve documentation for QQ.random_element.