CODEWITHSUNDEEP
pythonloops
thephfactor
thephfactor

Reputation: 181

Python nested for loop: what am I doing wrong?

I am working with data pulled from a spreadsheet-like file. I am trying to find, for each "ligand", the item with the lowest corresponding "energy". To do this I'm trying to make a list of all the ligands I find in the file, and compare them to one another, using the index value to find the energy of each ligand, keeping the one with the lowest energy. However, the following loop is not working out for me. The program won't finish, it just keeps running until I cancel it manually. I'm assuming this is due to an error in the structure of my loop.

for item in ligandList:
    for i in ligandList:
        if ligandList.index(item) != ligandList.index(i):
            if ( item == i ) :
                if float(lineList[ligandList.index(i)][42]) < float(lineList[ligandList.index(item)][42]):
                    lineList.remove(ligandList.index(item))
                else:
                    lineList.remove(ligandList.index(i))

As you can see, I've created a separate ligandList containing the ligands, and am using the current index of that list to access the energy values in the lineList.

Does anyone know why this isn't working?

Upvotes: 0

Views: 81

Answers (3)

Adam Smith
Adam Smith

Reputation: 54163

You look like you're trying to find the element in ligandList with the smallest value in index 42. Let's just do that....

min(ligandList, key=lambda x: float(x[42]))

If these "Ligands" are something you use regularly, STRONGLY consider writing a class wrapper for them, something like:

class Ligand(object):
    def __init__(self,lst):
        self.attr_name = lst[index_of_attr] # for each attribute
        ... # for each attribute
        ... # etc etc
        self.energy = lst[42]
    def __str__(self):
        """This method defines what the class looks like if you call str() on
it, e.g. a call to print(Ligand) will show this function's return value."""
        return "A Ligand with energy {}".format(self.energy) # or w/e
    def transmogfiscate(self,other):
        pass # replace this with whatever Ligands do, if they do things...

In which case you can simply create a list of the Ligands:

ligands = [Ligand(ligand) for ligand in ligandList]

and return the object with the smallest energy:

lil_ligand = min(ligands, key=lambda ligand: ligand.energy)

As a huge aside, PEP 8 encourages the use of the lowercase naming convention for variables, rather than mixedCase as many languages use.

Upvotes: 1

Ry-
Ry-

Reputation: 224858

That’s a really inefficient way of doing things. Lots of index calls. It might just feel infinite because it’s slow.

Zip your related things together:

l = zip(ligandList, lineList)

Sort them by “ligand” and “energy”:

l = sorted(l, key=lambda t: (t[0], t[1][42]))

Grab the first (lowest) “energy” for each:

l = ((lig, lin[1].next()[1]) for lig, lin in itertools.groupby(l, key=lambda t: t[0]))

Yay.

result = ((lig, lin[1].next()[1]) for lig, lin in itertools.groupby(
    sorted(zip(ligandList, lineList), key=lambda t: (t[0], t[1][42])),
    lambda t: t[0]
))

It would probably look more flattering if you made lineList contain classes of some kind.

Demo

Upvotes: 1

Hyperboreus
Hyperboreus

Reputation: 32429

It is a bit hard to answer without some actual data to play with, but I hope this works, or at least leads you into the right direction:

for idx1, item1 in enumerate(ligandList):
    for idx2, item2 in enumerate(ligandList):
        if idx1 == idx2: continue
        if item1 != item2: continue
        if float(lineList[idx1][42]) < float(lineList[idx2][42]):
            del lineList [idx1]
        else:
            del lineList [idx2]

Upvotes: 2

Related Questions