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

Dig Deeper for Nth Prime #3496

Merged
merged 1 commit into from Dec 18, 2023
Merged

Conversation

safwansamsudeen
Copy link
Contributor

No description provided.

@BethanyG
Copy link
Member

@Kiran1689, please see my comments on #3456.

@safwansamsudeen
Copy link
Contributor Author

@BethanyG just to give you a heads up - it's been a few months.

@BethanyG
Copy link
Member

@safwansamsudeen - Thanks. I have some work I need to wrap up on tooling, but will review this in the next week.

Copy link
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

Thank you for this. Apologies for the delay in reviewing/approving.

@BethanyG BethanyG merged commit 0b78195 into exercism:main Dec 18, 2023
16 checks passed
@safwansamsudeen
Copy link
Contributor Author

safwansamsudeen commented Dec 18, 2023

I recently realized while reviewing another person's solution that some bits of this code weren't great, though. This:

from itertools import islice, count

def is_prime(n):
    return not any(n % k == 0 for k in range(2, int(n ** 0.5) + 1))

def prime(number):
    if number == 0:
        raise ValueError('there is no zeroth prime')
    gen = islice(filter(is_prime, count(2)), number)
    for _ in range(number - 1): next(gen)
    return next(gen)

is much better off as:

from itertools import count

def is_prime(n):
    return all(n % k != 0 for k in range(2, int(n ** 0.5) + 1))

def prime(number):
    if number == 0:
        raise ValueError('there is no zeroth prime')
    gen = filter(is_prime, count(2))
    for _ in range(number - 1): next(gen)
    return next(gen)

Can I submit another PR, or shall we leave it as is?

@BethanyG
Copy link
Member

Let's leave it for now, plz. I have some track maintenance planned for EOM/ beginning of year, and we can address it then. 😄

@safwansamsudeen
Copy link
Contributor Author

@BethanyG did you address it? Or shall I open a PR?

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

Successfully merging this pull request may close these issues.

None yet

3 participants