"Sieve" instructions disagree with "Dig Deeper"

The instructions for the “Sieve” exercise state:

To check you are implementing the Sieve correctly, a good first test is to check that you do not use division or remainder operations.

However the “Dig Deeper” for Python offers the following code under “Approach: Using complex or nested comprehensions”

def primes(limit):
    return [number for number in range(2, limit + 1) if 
            all(number % divisor != 0 for divisor in range(2, number))]

This approach clearly uses the remainder operator and violates the instructions for the exercise.

Hi @xanni :wave:

Thanks for pointing that out. I’ll take a look when I get time, but it might not be quickly since we have the bootcamp and upgrade work to do on the track, as well as other maintenance.

The instructions are a bit vague and could be easily misinterpreted. The statement ‘do not use division or remainder operations’ should be understood as ‘do not use division or remainder operations for marking the numbers.’ However, this isn’t mentioned explicitly, so people might mistakenly assume they shouldn’t use these operations anywhere in the algorithm, even though using them for other calculations is allowed.

No problem. Thanks for looking into it!

In this case the remainder operation is indeed being used for marking the numbers, though.

Agreed.

I don’t have time right now to re-write the approaches docs and/or add an instruction append for clarity.

But I would welcome PRs to that end, if you’re game. :smile:

We could also launch a discussion around clarifying the core problem description/story. I know you are an expert in those kinds of clarifications. :smile: :blue_heart: .

True. However, the instructions might have misled the person who wrote this approach.

Clarifying the note is probably a better approach. Let’s keep this thread focused on the Python issue and use a separate thread for the instructions.

1 Like

Also, on completing the exercise in the Python track it suggests you have advanced your knowledge of sets, but that’s not necessarily the case since sets are not mentioned in the instructions nor required by common solutions. If that’s the intention, the Python track addendum should probably hint at the special advantages of using sets for a Python solution. However as the narrative suggests that the aim is to use as much CPU time as possible and the Sieve is inherently not the most efficient way to generate the primes I don’t find it in the spirit of the narrative to optimise the solutions for performance in any case!

@tasx and @xanni – I have some time today to take a look at this.

Do either of you have any proposals for a Python-specific instruction append? Please chime in. :smile:

Meanwhile, I will update the code in the approach to not use %.

Would the append address the issue of ‘do not use remainder or division’ operations? Or do you have something else in mind?

1 Like

Mainly that (since that was the main issue brought up in this thread) - but I would also welcome any other clarifications that would help and or challenge Pythonistas in particular. :smile:

Honestly, I can’t propose anything Python-specific, as I don’t have sufficient knowledge of the language. My only suggestion for now would be a modification of the instructions. I would change the note to:

The Sieve of Eratosthenes marks off multiples of each prime using addition (repeatedly adding the prime) or multiplication (directly computing its multiples), rather than checking each number for divisibility (using either division or remainder operations).

The tests don't check that you've implemented the algorithm, only that you've come up with the correct list of primes.

The wording here is very precise, so if you want to adopt parts of it for the Python track, any modifications should be made carefully. Alternatively, we can discuss potential changes to the current instructions in a new thread.

We might as well leave the “(using either division or remainder operations)” out since there are also other ways to check for divisibility (bit-shift).

1 Like

Let’s go ahead with that discussion in a separate thread. This exercise was featured during 48in24 so maintainers worked on updating the documentation beforehand. However, they had to stop early to make sure tracks had time to get the changes synced. This time, we don’t have a deadline to contend with.

2 Likes

Okay ^^

2 Likes

I’ve merged [Sieve]: Update content.md for Comprehension Approach by BethanyG · Pull Request #3865 · exercism/python · GitHub. It caveats the example with a warning about the % operator (that the instructions ask you not to use it).

I thought it better to point out that using said operator doesn’t really get you anywhere performance-wise, and its counter to the instructions rather than omit it from the article.

3 Likes