Atomic structures with all atoms deleted are not closed in Python

[Tom Goddard]

The atomic structure C++ code deletes the C++ structure automatically if all the atoms are deleted. But the Python code Structure.delete() does not get called and the Models.close([structure]) does not get called leaving a dead structure which will then cause errors if anything is done with it (for instance, saving a session). The typical result is a traceback with an AttributeError _c_pointer_ref or _c_pointer does not exist. We have 32 bug reports with _c_pointer_ref and over a 100 with _c_pointer although only some are attribute errors.

Here is a demonstration of the problem.

open 1a0m

Then from Python shell

session.models.list()[0].atoms.delete()

The structure is still shown in model panel and trying to save a session or show atoms raises attribute errors.

I think there are two approaches to fixing this error prone behavior. 1) make sure when the C++ destroys the structure that the Python is fully cleaned up (Structure.delete() called), or 2) don't have the C++ automatically destroy the structure when there are no atoms, the structure remains alive with 0 atoms and it is the job of Python code to check for this and remove the structure. I favor the second solution since deleting the structure as a side effect is too error prone -- for instance code might delete atoms then adding others which will work unless the the delete removes all the atoms in which case it will fail. Up to Eric to decide though.

This bug was the cause of morphing bug #5525 where the morph code checked if the morph structure had been deleted with Structure.deleted which incorrectly returned False because the Python Structure.delete() method was never called when C++ deleted the structure because it had no atoms.

[pett]

I tested out having the structure live on with zero atoms and immediately got a segmentation fault, and I'm pretty sure that was the tip of a very large iceberg -- so I went the other route and had deleting all atoms call Structure.delete() rather than directly calling the C++ destructor.

Fixed by ​this commit.

I only put it on the develop branch. If you felt it was a good/safe idea I could put it on the release branch as well.

[pett]

Had the wrong commit ID in the link. Corrected now.

[goddard@…]

I would not put that in 1.3 -- too dangerous.  Seems good for 1.4 though.