0

I'm new with python language and I'm having trouble to understand why this code doesn't work as I expect. I want to calculate and put in a tuple the primitive pythagorean triplet (a^2+b^2=c^2) for a,b,c < 100. This is the code

n=100
#Here I calculate all the pythagorean triples and I put it in a list(I wanted to use the nested list comprehension)
d=[(ap,b,c) for ap in range(1,n+1) for b in range(ap,n+1) for c in range(b,n+1)  if ap**2 +   b**2 == c**2  ]
#it work
#now I wonna find the primitive one: 
q=[]
for q in d: #I take each triples 
    #I check if it is primitive
    v=2       
    for v in range(2,q[0]) : 
        if q[0]%v==0 and q[1]%v==0 and q[2]%v== 0 :
            d.remove(q)  #if not I remove it and exit from this cycle
            break
#then I would expect that it read all the triples, but it doesn't
#it miss the triples after the one I have cancelled

Can you tell me why? Is there another way to solve it? Do I miss some step ?

Machavity
  • 30,841
  • 27
  • 92
  • 100

1 Answers1

1

The missing 3-tuples are not caused by the break but by modifying a list at the same time that you loop it. When you remove an element from a list, the remaining element's indexes is also modified, which can produce a loop to skip certain elements from being checked. It's never a good practice to remove elements from a list you are iterating. One usually create a copy of the list that you iterate, or use functions such as filter.

Also, you can remove v = 2. There's no need to set the value of v to 2 for every iteration when you already do so with the instruction for v in range(2,q[0])

Iterating over a copy of d

If you do list(d) then you create a clone of the list you are iterating. The original list can be modified but it won't be a problem because your loop will always iterate over the original list.

n=100
d=[(ap,b,c) for ap in range(1,n+1) for b in range(ap,n+1) for c in range(b,n+1)  if ap**2 + b**2 == c**2]
q=[]
for q in list(d):
    for v in range(2,q[0]) :
        if q[0]%v==0 and q[1]%v==0 and q[2]%v== 0 :
            d.remove(q)
            break

Use the function filter For the filter function , you need to define a function that is applied to every element of your list. The filter function uses that defined function to build a new list with the elements that pass. If the defined function returns True then that element is kept and used for the new list, if it returns False, then that element is not used. filter() returns an iterator, so you need to build the list from it.

def removePrimitives(q):
    for v in range(2, q[0]):
        if q[0] % v == 0 and q[1] % v == 0 and q[2] % v == 0:
            return False
    return True

n=100
d=[(ap,b,c) for ap in range(1,n+1) for b in range(ap,n+1) for c in range(b,n+1)  if ap**2 + b**2 == c**2]
q=[]
d = list(filter(removePrimitives, d))

Bonus: debugging

When it comes to coding, no matter what language, I believe one of the first things you should learn to do is how to debug it. Python has an amazing interactive debugging module called: ipdb.

Here's the commands:

n[ext]:         next line, step over
s[tep]:         step into next line or function.
c[ontinue]:     continue
l[ine]:         show more lines
p <variable>:   print value
q[uit]:         quit debugging
help:           show all commands
help <command>: show help

Install the package with your pip installer. Here's how you could have used it in your code to see exactly what happens when a primitive tuple is found and you break from the inner loop:

import ipdb
n=100
d=[(ap,b,c) for ap in range(1,n+1) for b in range(ap,n+1) for c in range(b,n+1)  if ap**2 +   b**2 == c**2  ]
q=[]
for q in d:
    for v in range(2,q[0]) :
        if q[0]%v==0 and q[1]%v==0 and q[2]%v== 0 :
            ipdb.set_trace() # This sets the breakpoint
            d.remove(q)
            break

At this breakpoint you can print the variable q, and d, and see how it gets modified and what happens after the break is executed.

Chayemor
  • 3,577
  • 4
  • 31
  • 54