schrodinger / pymol-open-source

Open-source foundation of the user-sponsored PyMOL molecular visualization system.
https://pymol.org/
Other
1.15k stars 275 forks source link

get_atom_coords() fails with object or selection name #322

Closed speleo3 closed 9 months ago

speleo3 commented 9 months ago

This fails:

cmd.pseudoatom("foo")
print(cmd.get_atom_coords("foo"))
 Error: Empty Selection

Works if passing a selection expression which is not an exact match with an object/selection name:

cmd.pseudoatom("foo")
print(cmd.get_atom_coords("(foo)"))
[0.0, 0.0, 0.0]

Reason seems to be the SelectorTmp::getAtomCount() function where m_count == 0 for named selections.

https://github.com/schrodinger/pymol-open-source/blob/d59041b68ce95fc9f72c54a3cce72437fe679b3e/layer3/Selector.h#L236

TstewDev commented 9 months ago

From looking at this very quickly, it seems like it's related to not wanting to create a temporary selection for an already named object/selection, leading to m_count being 0. I think I've observed this behavior in more complex examples of named selections not behaving how I would expect. I'll investigate further and make whatever changes result in the behavior that makes the most sense, but I'm hesitant to change anything immediately since this feels deliberate and may result in other issues if we're not careful.

speleo3 commented 9 months ago

No need to change anything immediately, I think this is a low priority bug (no crash and there is a workaround).

SelectorTmp::getAtomCount is only used once. You could consider removing it and calling SelectorCountAtoms() instead. Or call SelectorCountAtoms() in SelectorTmp::getAtomCount if m_count == 0. That would fix the issue.

Not creating a new temporary selection for a named selection makes sense. But the assumption that SelectorGetTmp() returns the atom count is not correct in that case.