Skip to content

Code refactoring needed (replace next() usage) #53

@dominiquesydow

Description

@dominiquesydow

I think we are misusing next() in this context (it works but it is not the proper way to do this and whenever errors occur the traceback is a bit difficult):

KinFragLib/kinfraglib/utils.py

Lines 1297 to 1302 in 99e94c2

dummy_1 = next(
atom for atom in combo.GetAtoms() if atom.GetProp("fragment_atom_id") == bond[0]
)
dummy_2 = next(
atom for atom in combo.GetAtoms() if atom.GetProp("fragment_atom_id") == bond[1]
)

What are we doing here?
We are selecting an atom from a molecule based on a fragment atom id (stored in bond). This selection should result in exactly one atom. Then, we are using next() to extract this one atom from the list comprehension. This is problematic in two ways:
(1) What if the selection returns no atom? Throws StopIteration I think.
(2) What if the selection returns multiple atoms? Should never happen but we should add a check.

What do I suggest?
Remove next(). Store selection in a list and go through all cases with if-elif-else (0, 1, >1 atoms), raise proper errors for 0 and >1 atoms.

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions