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

random_prime is badly documented. #10111

Closed
sagetrac-drkirkby mannequin opened this issue Oct 9, 2010 · 6 comments
Closed

random_prime is badly documented. #10111

sagetrac-drkirkby mannequin opened this issue Oct 9, 2010 · 6 comments

Comments

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Oct 9, 2010

The random_prime() function in Sage can take one, two or three arguments. But all the examples shown in the documentation or random_prime use only one argument. The more complex cases are not documented and not tested.

See also http://groups.google.com/group/sage-devel/browse_thread/thread/6e8d6f28c915830d?hl=en

These are the examples given.

        sage: random_prime(100000)
        88237
        sage: random_prime(2)
        2

Although some with good Python knowledge may argue the behavior with 2 or 3 arguments is documented properly, I personally think it could be clearer.

For example, I'm told:

random_prime(123,False)

would normally be written as

random_prime(123, proof=False)

It would be good with someone with decent Python knowledge to write some examples of using this function with 2 or 3 arguments.

See also #10112, where it is shown that the function hangs for certain erroneous inputs.

It should also be noted that the error message "*n must be greater than lbound *" is incorrect, and should be changed to "n must be at least lbound"

Fixed by #10112, please close as duplicate.

CC: @sagetrac-fwclarke @burcin

Component: documentation

Reviewer: Francis Clarke

Issue created by migration from https://trac.sagemath.org/ticket/10111

@sagetrac-drkirkby sagetrac-drkirkby mannequin added this to the sage-5.0 milestone Oct 9, 2010
@sagetrac-drkirkby

This comment has been minimized.

@mwhansen
Copy link
Contributor

comment:2

See the patch at #10112.

@sagetrac-drkirkby

This comment has been minimized.

@sagetrac-drkirkby
Copy link
Mannequin Author

sagetrac-drkirkby mannequin commented Mar 28, 2011

comment:3

Since there appears to be some disagreement about the code changes on #10112, if those can't be resolved soon, I suggest we just change the documentation on this ticket. The 3 changes needed are:

  • The error message "*n must be greater than lbound *" is incorrect, and should be changed to "n must be at least lbound"
  • The example random_prime(200, lbound=100) is added
  • The example random_prime(200, proof=False, lbound=100) is added.

@sagetrac-fwclarke

This comment has been minimized.

@jdemeyer
Copy link

Reviewer: Francis Clarke

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants