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 , 8 years ago
Owner: | changed from | to
---|
comment:2 by , 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 , 8 years ago
Status: | assigned → accepted |
---|
follow-up: 4 comment:4 by , 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 , 8 years ago
Cc: | 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 , 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 , 8 years ago
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
Fixed the model-panel issues of this ticket.
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.