parent model needs explicit test against None

[Tristan Croll]

The following bug report has been submitted:
Platform:        Linux-3.10.0-957.12.2.el7.x86_64-x86_64-with-centos-7.6.1810-Core
ChimeraX Version: 0.91 (2019-11-28)
Description
This made me spend some time tearing my hair out and cursing Python...

In core.models.Models.add(), the following:

        if parent:
            for m in models:
                if m.parent is None or m.parent is not parent:
                    parent.add_drawing(m)

... breaks when parent implements a __len__() method and len(parent)==0. The added models are actually placed in the tree with the correct IDs, but their parent is 'root' and the *desired* parent knows nothing about them. 

I can work around it by adding a __bool__() method that always returns True, but it would probably be for the best if the test were 'if parent is not None:'

Log:
Startup Messages  
---  
warning | 'clip' is a prefix of an existing command 'clipper'  
  
UCSF ChimeraX version: 0.91 (2019-11-28)  
© 2016-2019 Regents of the University of California. All rights reserved.  
How to cite UCSF ChimeraX  

> open /run/media/tic20/storage/structure_dump/test_session_save/test.cxs

Summary of feedback from opening
/run/media/tic20/storage/structure_dump/test_session_save/test.cxs  
---  
notes | Adding Drawing atoms to structure  
Adding Drawing ribbon to structure  
Adding Drawing 3io0 to Data manager (3io0)  
Adding Drawing Map Manager to Data manager (3io0)  
Adding Drawing Crystallographic maps (3io0-sf.cif) to Map Manager  
Adding Drawing Reflection Data to Crystallographic maps (3io0-sf.cif)  
Adding Drawing Free flags to Reflection Data  
Adding Drawing Experimental to Reflection Data  
Adding Drawing Calculated to Reflection Data  
Adding models: ['Data manager (3io0)']  
Parent is None!  
Parent name: root  
Adding Drawing Data manager (3io0) to root  
Adding models: ['3io0', 'Map Manager']  
Parent name: Data manager (3io0)  
Adding models: ['Crystallographic maps (3io0-sf.cif)']  
Parent name: Map Manager  
Adding models: ['Reflection Data']  
Parent name: Crystallographic maps (3io0-sf.cif)  
Adding models: ['Free flags', 'Experimental', 'Calculated']  
Parent name: Reflection Data  
Adding Drawing Depth Indicator to root  
Adding models: ['intensity_meas, intensity_sigma']  
Parent is None!  
Parent name: root  
Adding Drawing intensity_meas, intensity_sigma to root  
Adding Drawing Pivot indicator to root  
Adding models: ['Atomic symmetry']  
Parent name: Data manager (3io0)  
Adding Drawing Atomic symmetry to Data manager (3io0)  
Adding Drawing Symmetry atoms to Atomic symmetry  
Adding Drawing Symmetry bonds to Atomic symmetry  
Adding Drawing Symmetry ribbons to Atomic symmetry  
  
opened ChimeraX session  
Adding Drawing 3io0 /A PRO 76 ribbons to ribbon  
Adding Drawing 3io0 #1.1 ribbon_tethers to ribbon  
Adding Drawing bonds to 3io0  
Adding Drawing 3io0 /A PRO 76 ribbons to Symmetry ribbons  
Adding Drawing 3io0 #1.1 ribbon_tethers to Symmetry ribbons  
Adding Drawing 3io0 /A PRO 76 ribbons to Symmetry ribbons  
Adding Drawing 3io0 #1.1 ribbon_tethers to Symmetry ribbons  

> toolshed show Shell

/opt/UCSF/ChimeraX-daily/lib/python3.7/site-
packages/IPython/core/history.py:226: UserWarning: IPython History requires
SQLite, your history will not be saved  
warn("IPython History requires SQLite, your history will not be saved")  




OpenGL version: 3.3.0 NVIDIA 418.87.01
OpenGL renderer: TITAN Xp/PCIe/SSE2
OpenGL vendor: NVIDIA Corporation

[Eric Pettersen]

Also happens in Models.remove()

[Tom Goddard]

Fixed.

You are really asking for problems adding len() method treating a model like a sequence. You will likely be burned by this again. You should probably use a method like "number_of_x()" instead of defining len().

[Tristan Croll]

Yes, I can quite easily do without it - to be honest, I don’t even know if I use it in the first place! The code in question is at least 2 years old, and only blew up now while I was working on session save/restore.