My function to filter out prime numbers is printing duplicates, how can I fix this?

Question

Here is what I have. The function works perfectly fine at filtering out composite numbers, but I can not figure out why it is printing duplicates of certain numbers:

def prime_filter(list):
    prime_list = []
    for i in list:
        if i == 2:
            prime_list.append(i)
        if i > 1:
            for n in range(2, i):
                if(i % n == 0):
                    break
                else:
                    prime_list.append(i)
    print("The total number of prime numbers is: ", len(prime_list))
    print(prime_list)

Here is my test of the function and the output it returns:

# test
test_list = [2, 3, 6, 8, 11]
prime_filter(test_list)

The total number of prime numbers is:  11
[2, 3, 11, 11, 11, 11, 11, 11, 11, 11, 11]

Anyone know how I can fix this? Any help is greatly appreciated.

first thing -> do not use `list` as avariable as its a reserved function in python — Matiiss, Oct 28 '20 at 18:39

score 2 · Answer 1 · answered Dec 22 '20 at 03:58

You should check whether prime_list is divided by all the numbers or not.

def prime_filter(list):
    prime_list = []
    for i in list:
        if i == 2:
            prime_list.append(i)
        if i > 2:
            for n in range(2, i):
                if(i % n == 0):
                    break
            else:
                prime_list.append(i)
                
    print("The total number of prime numbers is: ", len(prime_list))
    print(prime_list)

Jaden · Answer 2 · 2020-10-28T18:55:13.347

1

This might work fine for the solution.i should be added to prime_list when all the check is done.

def prime_filter(list):
    prime_list = []
    for i in list:
        if i == 2:
            prime_list.append(i)
        if i > 1:
            for n in range(2, i):
                if(i % n == 0):
                    break
                if n**2>i:
                    prime_list.append(i)
                    break
    print("The total number of prime numbers is: ", len(prime_list))
    print(prime_list)

edited Oct 28 '20 at 18:55

answered Oct 28 '20 at 18:41

Jaden

192
4
15

This is so called `Sieve of Eratosthenes` algorithm. – Jaden Oct 28 '20 at 18:56

score 1 · Answer 3 · answered Oct 28 '20 at 18:51

in your code for i>1 for every value of i which is odd is getting appended as odd no%2==1 which makes the program go in else part.

also, you only need to add else part when using for loop no value divides the i.

here is improved working code

def prime_filter(_list):
    prime_list = []
    for i in _list:
        if i == 2:
            prime_list.append(i)
        elif i==1:
            continue
        elif i > 2:
            for n in range(2, i):
                if(i % n == 0):
                    break
            else:
                prime_list.append(i)
    print("The total number of prime numbers is: ", len(prime_list))
    print(prime_list)

test_list = [2, 3, 6, 8, 11]
prime_filter(test_list)
# output [2,3,11]

how to check a no is prime or not

Marat · Answer 4 · 2020-10-28T19:10:45.313

1

sahasrara62 and Abhinav answers are, of course, correct, but I would like to point out the bizzare inefficiency here. The biggest factor of composite n is at most sqrt(n), which allows to save many iterations at the inner loop.

Another optimization is to avoid checking even factors - we already know the value is odd

def prime_filter(lst):
    prime_list = [value for value in lst
                  if (not value % 2 
                      or any(not value % n for n in range(3, value**.5 + 1, 2))
                     )]

It can be further improved by adding quick primality test before checking factors

edited Oct 28 '20 at 19:10

answered Oct 28 '20 at 18:56

Marat

15,215
2
39
48

I would've recommended Sieve of Eratosthenes for efficiency. But OP needed to figure out the error rather than improve the speed/complexity of it – Abhinav Mathur Oct 28 '20 at 18:57
@AbhinavMathur SE is a slightly different thing, it finds all prime numbers less than a given one. Here we are only looking at primality test, which is a different thing. – Marat Oct 28 '20 at 19:00
Primality testing for a large number of queries becomes much less efficient than SE, it depends on the order of numbers to be tested – Abhinav Mathur Oct 28 '20 at 19:01
@AbhinavMathur Yep, space vs computational complexity tradeoff. But unlike RAM capacity, CPU time is (potentially) unlimited – Marat Oct 28 '20 at 19:02
Depends on the usage. If the intended use is for competitive programming, then time is far more important than space in most cases – Abhinav Mathur Oct 28 '20 at 19:04
@AbhinavMathur good luck handling this case: `lst = [10**10]` – Marat Oct 28 '20 at 19:07
it's better then mine :D – sahasrara62 Oct 28 '20 at 19:08
I never said SE is the solution to everything. But most such questions are created to use that concept – Abhinav Mathur Oct 28 '20 at 19:09
@AbhinavMathur You implement algos based on the situation not on whether it is best or not – sahasrara62 Oct 28 '20 at 19:09

score 1 · Answer 5 · answered Oct 28 '20 at 18:57

Your code won't get prime correctly, it will add all numbers not divided by 2, e.g. if there is 9 in the list, it will be included in prime_list. I would like to suggest the following:

def prime_filter(list):
prime_list = []
for i in list:
    if i == 2:
        prime_list.append(i)
    if i > 2:
        prime_flag = True
        for n in range(2, i):
            if i % n == 0:
                prime_flag = False
                break

        if prime_flag:
            prime_list.append(i)
print("The total number of prime numbers is: ", len(prime_list))
print(prime_list)

test_list = [2, 3, 6, 8, 9, 11] prime_filter(test_list)

The total number of prime numbers is: 3

[2, 3, 11]

funfact you can use `else` after `for` loop , as no need to have a check to see if condition satisfied or not — sahasrara62, Oct 28 '20 at 19:10

Abhinav Mathur · Answer 6 · 2020-10-28T18:56:22.810

else:
    prime_list.append(i)

For every factor that doesn't divide i, it will be appended to the list, which is incorrect. Just change the function to

def prime_filter(lst):
    prime_list = []
    for i in lst:
        isprime = 1
        if i == 2:
            prime_list.append(i)
        else if i > 1:
            for n in range(2, i):
                if(i % n == 0):
                    isprime = 0
                    break
            if isprime:
                prime_list.append(i)
    print("The total number of prime numbers is: ", len(prime_list))
    print(prime_list)

My function to filter out prime numbers is printing duplicates, how can I fix this?

6 Answers6