Opened 8 years ago

Closed 8 years ago

#699 closed defect (fixed)

core.commands.rename issues

Reported by: Tristan Croll Owned by: pett
Priority: minor Milestone:
Component: Sessions Version:
Keywords: Cc: Tom Goddard
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

If I create an empty model first:

from chimerax.core.models import Model
m = Model('test', session)
session.models.add([m])

... then load a structure via the gui, and in the command line do:

rename #2 id 1

... then I get the following error:

Error processing trigger "frame drawn"
Traceback (most recent call last):
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/triggerset.py", line 126, in invoke
return self._func(self._name, data)
File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/model_panel/tool.py", line 147, in _fill_tree
item.setText(self.ID_COLUMN, model_id_string)
AttributeError: 'NoneType' object has no attribute 'setText'

If I load two structures after adding the Model to the session and do

rename #3 id 1

... or the structure is loaded before the Model is added to the session and I do

rename #1 id 2

... then it seems to work fine.

Also,

rename #{n} newname

... fails to update the name in the Models panel.

Finally, a request: would it be possible to add a function essentially equivalent to change_model_id, that allows re-parenting to a model that hasn't yet been added to the session? I'm using the session.triggers 'add models' trigger in downstream code, and that expects the hierarchy under the new parent model to be complete at the point it's added to the session.

Change History (7)

comment:1 by Tom Goddard, 8 years ago

Owner: changed from Tom Goddard to pett

The changes of id number seem to work, only Model Panel throws an error when it tries to update its list. Closing model panel first eliminates the error.

May need more work for model panel to update when a model name changes. For volumes and other models there is no trigger fired when the name changes so some trigger probably needs to be added so Model Panel knows to update.

Reassigning to Eric to handle these Model Panel issues.

comment:2 by Tom Goddard, 8 years ago

For the final request about reparenting models that are not yet added to the session, can you not just do something like

parent.remove_drawing(child, delete = False)
new_parent.add([child])

If there is more subtlety than that, or you want a routine for those two lines please provide code for what you want.

comment:3 by pett, 8 years ago

Status: assignedaccepted

in reply to:  4 ; comment:4 by tic20@…, 8 years ago

I think something more than this would be useful for two reasons:

1. Using a function called remove_drawing() to disconnect a Model object 
from its parent is unintuitive to anyone who isn't intimately familiar 
with the inner workings of ChimeraX. A convenience function named 
something like m.transplant(new_parent) would be more immediately 
understandable, in my opinion.

2. If I apply the below, where m is model 1.1 and newm is a 
newly-created model not yet added to the session:

m.parent.remove_drawing(m, delete = False)
newm.add([m])
session.model.add([newm])

newm.id_string()
   '2'

m.id_string()
   '1.1'

That's why I settled on the move_model() function I posted earlier 
(modified and pasted below). It's put together based on the internal 
logic of existing functions in the Drawing and Model classes, and takes 
care of the above issue (that is, makes sure the model and all its 
children are renumbered correctly) as well as a few other things:

- turning off _auto_style for the moved model to stop it from changing 
the display settings when it's re-added to the session
- preventing the user from accidentally making the model subordinate to 
itself (not sure what the consequences of that would be, but I doubt 
they'd be pretty)
- freeing up the old model id for re-use

If the below is added to the Model class, then:
m.transplant(newm)

... works correctly whether newm is already part of the session or not 
(that is, its tree remains intact, its display is exactly as it was 
before, and everything is immediately updated in the Models panel). 
There may well be a more efficient way to do it, but I think a named 
function with equivalent functionality would still be a worthwhile 
thing.



     def transplant(self, new_parent):
         '''
         Removes this model from the ChimeraX model tree and transplants
         it (with all its children intact) as the child of a different 
model.
         '''
         self._auto_style = False
         mlist = self.all_models()
         model_id = self.id
         session = self.session
         if new_parent in mlist:
             raise RuntimeError('Target model cannot be one of the models 
being moved!')
         for m in mlist:
             m.removed_from_session(session)
             mid = m.id
             if mid is not None:
                 del session.models._models[mid]
                 m.id = None
         session.triggers.activate_trigger('remove models', mlist)
         if len(model_id) == 1:
             parent = session.models.drawing
         else:
             parent = session.models._models[model_id[:-1]]
         parent.remove_drawing(self, delete=False)
         parent._next_unused_id = None
         new_parent.add([self])




On 2017-06-01 22:05, ChimeraX wrote:

comment:5 by pett, 8 years ago

Cc: Tom Goddard added

The issues that Tristan raises in his last comment have nothing to do with Model Panel, so I am adding Tom to the cc list so that he can look at them and respond.

comment:6 by Tom Goddard, 8 years ago

The model reparenting behavior you observe is a bug. I've fixed it. Also you no longer need to remove a model from its parent. SImply adding it to a new parent will take care of that. So reparenting to a newly created model can be done with

newm.add([m])
session.model.add([newm])

The added_to_session() call will not be done on the a reparented model that was already added to the models list, so no need to worry about auto styling.

comment:7 by pett, 8 years ago

Resolution: fixed
Status: acceptedclosed

Fixed the model-panel issues of this ticket.

Note: See TracTickets for help on using tickets.