introduce primes() to generate kth to nth prime numbers #20830

sidhu1012 · 2021-01-20T08:46:24Z

References to other Issues or PRs

Fixes #19118

Brief description of what is fixed or changed

Other comments

Release Notes

ntheory
- Introduce primes() to generate kth to nth prime numbers or first n prime numbers.

sympy-bot · 2021-01-20T08:46:25Z

✅

Hi, I am the SymPy bot (v161). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

ntheory
- Introduce primes() to generate kth to nth prime numbers or first n prime numbers. (#20830 by @sidhu1012)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.9.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #19118

#### Brief description of what is fixed or changed


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* ntheory
    * Introduce `primes()` to generate kth to nth prime numbers or first n prime numbers.
<!-- END RELEASE NOTES -->

sidhu1012 · 2021-01-20T11:52:33Z

Ready for review.

asmeurer · 2021-04-12T23:27:14Z

As I noted on the issue, I support this in principle. However, I think the API here needs work

nprime isn't a great name. If anything, it should be pluralized (nprimes). I don't know if we can do better than that.
Having a two argument form is nice, but I think if two arguments are provided, they should be reversed (similar to the built-in range), so that they are in order.
I think the two argument form should be inclusive, so nprimes(k, n) should return a list of n - k + 1 elements. Your current docstring says "both inclusive" but the docstring example shows k=3 not included in the list.

asmeurer · 2021-04-12T23:28:09Z

sympy/ntheory/generate.py

@@ -1005,3 +1005,34 @@ def compositepi(n):
    if n < 4:
        return 0
    return n - primepi(n) - 1
+
+def nprime(nth, kth=1):


The parameters should just be n and k.

@asmeurer With parameters being just n and k people may get confused like you did #20830 (comment)

asmeurer · 2021-04-12T23:28:20Z

sympy/ntheory/generate.py

+    """
+    n, k = map(int, (nth, kth))
+    if n < 1:
+        raise ValueError("nth must be a positive integer; nprime(1) == 2")


The example at the end of the error message is confusing.

asmeurer · 2021-04-12T23:28:44Z

sympy/ntheory/generate.py

+    if k < 1:
+        raise ValueError("kth must be a positive integer; nprime(3, 2) == [3, 5]")
+    if k>n:
+        raise ValueError("kth can't be greater than nth")


I this case it could just return an empty list.

asmeurer · 2021-04-12T23:28:56Z

sympy/ntheory/generate.py

+    >>> nprime(7, 3)
+    [5, 7, 11, 13, 17]
+    """
+    n, k = map(int, (nth, kth))


Use as_int here.

asmeurer · 2021-04-12T23:30:52Z

Your current docstring says "both inclusive" but the docstring example shows k=3 not included in the list.

Oh never mind I was confusing the kth prime with the prime k. You might want to adjust the example so it is clear it is referring to the kth primes and not k as a prime. I would suggest using larger inputs for the 2-arg example, like nprimes(20, 25).

asmeurer · 2021-04-12T23:31:23Z

sympy/ntheory/generate.py

@@ -1005,3 +1005,34 @@ def compositepi(n):
    if n < 4:
        return 0
    return n - primepi(n) - 1
+
+def nprime(nth, kth=1):
+    """ Returns kth to nth prime numbers(both inclusive).


Follow the docstring style guide. https://docs.sympy.org/latest/documentation-style-guide.html

smichr · 2021-04-15T12:32:22Z

Please see issue page #19118 for alternate solution using existing prime with two arguments.

smichr · 2021-04-15T13:53:06Z

sympy/ntheory/generate.py

+    if k>n:
+        return []


Suggested change

if k>n:

return []

if k > n:

raise ValueError('k should not be less than n')

smichr · 2021-04-15T13:54:52Z

sympy/ntheory/tests/test_generate.py

+    raises(ValueError, lambda: nprimes(0))
+    raises(ValueError, lambda: nprimes(10, 0))
+    raises(ValueError, lambda: nprimes(10,-5))
+    assert nprimes(10, 2) == []


since this is not working like a range function I think we can be more strict in requiring that k >=n .

Also, I like the name primes better.

@smichr nprimes seems to be better.

The n does not provide more clarity than the pluralizing s. I prefer one of the two options I described in the issue and am barely +1/2 because there is already a clear way to do this with primerange. But something that makes me close to +1 is getting a list rather than a generator: list(primerange(prime(10)+1)) vs primes(10).

I'll make it primes

sympy/ntheory/generate.py

Co-authored-by: Christopher Smith <smichr@gmail.com>

smichr · 2021-04-21T11:41:23Z

@oscarbenjamin , I imagine that you would prefer a separate function like this instead of a polymorphic prime which gave the nth prime for 1 argument and a list of primes for the two argument case? e.g. prime(1) -> 2, prime(1,1) -> [2], prime(1,3) -> [2,3,5]

@sidhu1012 , documentation showing how to get a list of the first n primes in other functions should be updated...I think you can regex search for primerange.*prime\(.

oscarbenjamin · 2021-04-21T18:55:33Z

It seems to me that the first issue is the documentation so I wouldn't say that this "fixes" the linked issue. It's clear that the documentation needs to be improved but it's not clear that we need this function. As @smichr says this is already quite doable with the existing primitives. The most useful thing we could add are "see also" references to all related functions linking to each of the other functions and examples in the docstrings showing how to do common things that users would want to do. If we had those then I'm not sure that there would be any need for this function.

Also is primes(k, n) actually any more efficient than primes(n)[k:]? If not then I don't see the benefit in providing it. The docstring can just show how to slice the list.

smichr · 2021-04-21T19:46:30Z

Also is primes(k, n) actually any more efficient than primes(n)[k:]

The primes function uses 1-based indexing of primes so primes(k,n) == primes(n)[k-1:]...which is ackward.
The only thing that bumps me to +1/2 is that getting a list of primes seems a little awkward; but one can always write their own convenience function if it is important to them.

smichr · 2021-04-21T19:46:47Z

ping @asmeurer who had some favorable inclinations

oscarbenjamin · 2021-04-21T19:55:36Z

The primes function uses 1-based indexing of primes so primes(k,n) == primes(n)[k-1:]...which is ackward.

But why does anyone specifically need this one-based indexing? It is better I think for sympy to just give something simple that is easy to understand and useful e.g. primes(n) -> list of first n primes. Then the user can use the rich facilities of Python to do whatever they want with that list.

If the function does something clever to compute primes(k, n) more efficiently than primes(n) then that's different. As far as I can tell this just computes primes(n) and returns a slice of it. Why bother having an extra parameter and the docs and tests and all the rest of it when the user can do whatever slicing they want?

smichr · 2021-04-21T20:06:23Z

do whatever slicing they want?

agree. And primes(k,n) == primes(n)[k-n:]

My only concern is that given this method people start using some bad habits. I'm not sure how those would play out but others may think of some. So I am slow to commit on this one.

smichr · 2021-04-21T21:11:00Z

write their own convenience function

>>> primes = lambda k,n=1: list(primerange(prime(min(k,n)), prime(max(k,n))+1))
>>> primes(4)
[2, 3, 5, 7]
>>> primes(3,4)
[5, 7]
>>> primes(4,3)
[5, 7]

introduce nprime() to generate kth to nth prime numbers

0b603bc

Fix docsrting

a3f02c5

czgdp1807 added the ntheory label Jan 21, 2021

asmeurer reviewed Apr 12, 2021

View reviewed changes

sympy/ntheory/generate.py Outdated

>>> nprime(7, 3)

[5, 7, 11, 13, 17]

"""

n, k = map(int, (nth, kth))

Copy link

Member

asmeurer Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use as_int here.

asmeurer reviewed Apr 12, 2021

View reviewed changes

sidhu1012 added 3 commits April 15, 2021 11:58

Update docstring of nprimes()

93fba41

Merge https://github.com/sympy/sympy into nprime

322d955

Remove trailing whitespace

4f80c07

sidhu1012 requested a review from asmeurer April 15, 2021 06:46

sidhu1012 mentioned this pull request Apr 15, 2021

Generating first n prime numbers #19118

Open

smichr reviewed Apr 15, 2021

View reviewed changes

make nprime stricter

b925e88

sidhu1012 requested a review from smichr April 19, 2021 06:57

smichr reviewed Apr 19, 2021

View reviewed changes

sympy/ntheory/generate.py Outdated Show resolved Hide resolved

smichr reviewed Apr 19, 2021

View reviewed changes

sympy/ntheory/generate.py Outdated Show resolved Hide resolved

smichr reviewed Apr 19, 2021

View reviewed changes

sympy/ntheory/generate.py Outdated Show resolved Hide resolved

smichr reviewed Apr 19, 2021

View reviewed changes

sympy/ntheory/generate.py Outdated Show resolved Hide resolved

sidhu1012 and others added 4 commits April 19, 2021 22:23

Update sympy/ntheory/generate.py

2111162

Co-authored-by: Christopher Smith <smichr@gmail.com>

Update sympy/ntheory/generate.py

22d4557

Co-authored-by: Christopher Smith <smichr@gmail.com>

Update sympy/ntheory/generate.py

224e25c

Co-authored-by: Christopher Smith <smichr@gmail.com>

Update sympy/ntheory/generate.py

8ce8a41

Co-authored-by: Christopher Smith <smichr@gmail.com>

sidhu1012 changed the title ~~introduce nprime() to generate kth to nth prime numbers~~ introduce primes() to generate kth to nth prime numbers Apr 20, 2021

Change nprimes() to primes()

5fd39fa

sidhu1012 requested a review from smichr April 20, 2021 07:35

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce primes() to generate kth to nth prime numbers #20830

introduce primes() to generate kth to nth prime numbers #20830

sidhu1012 commented Jan 20, 2021 •

edited

sympy-bot commented Jan 20, 2021 •

edited

sidhu1012 commented Jan 20, 2021

asmeurer commented Apr 12, 2021

asmeurer Apr 12, 2021

sidhu1012 Apr 13, 2021

asmeurer Apr 12, 2021

asmeurer Apr 12, 2021

asmeurer Apr 12, 2021

asmeurer commented Apr 12, 2021

asmeurer Apr 12, 2021

smichr commented Apr 15, 2021 •

edited

smichr Apr 15, 2021

smichr Apr 15, 2021

sidhu1012 Apr 17, 2021

smichr Apr 19, 2021

sidhu1012 Apr 19, 2021

smichr commented Apr 21, 2021

oscarbenjamin commented Apr 21, 2021

smichr commented Apr 21, 2021

smichr commented Apr 21, 2021

oscarbenjamin commented Apr 21, 2021

smichr commented Apr 21, 2021

smichr commented Apr 21, 2021

-    if k>n:
-        return []
+    if k > n:
+        raise ValueError('k should not be less than n')

introduce primes() to generate kth to nth prime numbers #20830

introduce primes() to generate kth to nth prime numbers #20830

Conversation

sidhu1012 commented Jan 20, 2021 • edited

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

sympy-bot commented Jan 20, 2021 • edited

sidhu1012 commented Jan 20, 2021

asmeurer commented Apr 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmeurer commented Apr 12, 2021

Choose a reason for hiding this comment

smichr commented Apr 15, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smichr commented Apr 21, 2021

oscarbenjamin commented Apr 21, 2021

smichr commented Apr 21, 2021

smichr commented Apr 21, 2021

oscarbenjamin commented Apr 21, 2021

smichr commented Apr 21, 2021

smichr commented Apr 21, 2021

sidhu1012 commented Jan 20, 2021 •

edited

sympy-bot commented Jan 20, 2021 •

edited

smichr commented Apr 15, 2021 •

edited