#683 closed defect (fixed)
atomspec_has_atoms() is inconsistently applied.
| Reported by: | Tristan Croll | Owned by: | Tom Goddard |
|---|---|---|---|
| Priority: | major | Milestone: | |
| Component: | Depiction | Version: | |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: | ||
| Notify when closed: | Platform: | all | |
| Project: | ChimeraX |
Description
Context: I'm sub-classing AtomicStructure to provide a visualisation of atom positional restraints, with Pseudobonds between true atom and target. I need to take control over visualisation of atoms for this object away from the main UI (that is, make its atoms invisible to AtomSpec).
AtomicStructure has the methods atomspec_has_atoms() and atomspec_atoms() which appear to be designed for this purpose, but they're a little inconsistently applied at the moment. If my subclass of AtomicStructure looks like:
class PositionRestraintIndicator(AtomicStructure):
'''
For each position restraint, we want a neat way to display its target
position with a bond back to the atom itself. Redrawing needs to be
fast, since at times there will be many restraints in play at once.
The simplest way to accomplish this is to work with the pre-existing
infrastructure for fast handling of atoms and bonds, and build the
targets as fake Atoms. This allows us to use the existing Pseudobonds
functionality to draw the bonds between atoms and their targets, giving
us a lot of functionality for free. However, it does mean we need to
subclass AtomicStructure, since we need to take all control over
visualisation away from the main ChimeraX interface.
'''
def atomspec_has_atoms(self):
return False
def atomspec_atoms(self):
return None
... then the atoms within will be ignored by the command-line interface, sure enough - but still treated as bona fide atoms by everything else. So, for example, if I type:
show selAtoms
at the cli with nothing selected, the atoms will still be shown. If I select some with the mouse, then:
from chimerax.core.atomic import selected_atoms sel = selected atoms(session)
... includes these atoms in the returned array, etc.. I think it would be very valuable if atomspec_has_atoms() were respected across the board, to provide for some much more flexible use of AtomicStructure. The changes I've made so far seem to work without breaking anything:
chimerax.core.objects:
line 61
def invert(self, session, models):
from .atomic import Structure, Atoms, concatenate
matoms = []
from .orderedset import OrderedSet
imodels = OrderedSet()
for m in models:
if isinstance(m, Structure):
< matoms.append(m.atoms)
> if m.atomspec_has_atoms():
> matoms.append(m.atomspec_atoms())
elif m not in self._models:
imodels.add(m)
iatoms = concatenate(matoms, Atoms, remove_duplicates=True) - self.atoms
imodels.update(iatoms.unique_structures)
self._atoms = [iatoms]
self._cached_atoms = iatoms
self._models = imodels
chimerax.core.structure:
line 1637:
def selected_items(self, itype):
if itype == 'atoms':
< atoms = self.atoms
> atoms = self.atomspec_atoms()
< if atoms.num_selected > 0:
> if atoms is not None and atoms.num_selected > 0:
return [atoms.filter(atoms.selected)]
return []
line 2543:
def structure_atoms(structures):
'''Return all atoms in specified atomic structures as an :class:`.Atoms` collection.'''
from .molarray import concatenate, Atoms
< atoms = concatenate([m.atoms for m in structures], Atoms)
> atoms = concatenate([m.atomspec_atoms() for m in structures if m.atomspec_has_atoms()], Atoms)
return atoms
With the above changes, a PositionRestraintIndicator model can still have its atoms individually shown/hidden via standard scripting, e.g.:
m.atoms.displays = True
... but is otherwise ignored by any atom-directed commands from the interface, which is what I need. I can't see any other way to achieve this without a lot more messing around.
Attachments (1)
Change History (15)
comment:1 by , 8 years ago
comment:2 by , 8 years ago
In my experience using fake atoms, much of the power is that the user can operate on them (measure distances, set their color and size). Maybe your problems relate to accidentally operating on them, and that may be solved by giving them a separate top level model id number.
comment:3 by , 8 years ago
My key issue is the top-left "show/hide atoms" buttons on the shortcut bar, which run "{show/hide} selAtoms" behind the scenes. At present, if a model has atomspec_has_atoms() returning False then this will work as expected as long as some atoms are selected: any selected atoms within this model will be ignored. But when no atoms are selected, then atomspec_has_atoms() is ignored: all atoms are collected in structures.structure_atoms:
atoms = concatenate([m.atoms for m in structures], Atoms)
... and therefore shown/hidden along with everything else. This is essentially all I'm looking to avoid.
comment:4 by , 8 years ago
... sorry, realised the above is confusing things further. That's not how it behaves by default: the selected atoms will be affected along with the rest. Momentarily forgot what I'd changed.
To summarise what I'm after: it would be nice to have a flag that tells ChimeraX, "these objects may be using the Atoms infrastructure but they're not actually atoms, and should be ignored by shortcut commands aimed at true atoms (e.g. show/hide atoms/bonds/ribbons)." That would provide a lot of added flexibility in terms of writing plugins, and I think would be more intuitive for the user: from their perspective these things clearly aren't atoms, just annotations - so why should they be affected by commands aimed at manipulating molecule display? The atomspec_has_atoms() seems a natural fit for such a flag, and with the changes made in my original ticket (which could easily be re-written to only use atomspec_has_atoms() and not atomspec_atoms()) does the job nicely with no nasty side-effects that I've come across.
by , 8 years ago
| Attachment: | image4.png added |
|---|
comment:5 by , 8 years ago
The attached gives an idea of what I'm going for, where the two pins are PositionRestraintIndicator "atoms" representing the restraint position (I overrode _update_level_of_detail() to point to the new drawing rather than sphere_geometry()). What I've been hoping to avoid is the need to create/destroy new atoms/pseudobonds on the fly: it's far easier to create a model with copies of all restrainable atoms (and their pseudobonds) once, and then just display the corresponding pin when a restraint is activated. I need to be able to also tie their display to the display of the "true" atom - it wouldn't be particularly ideal to have restraints still shown when the atom itself is hidden. What I of course need to avoid is having the user accidentally show all of them - this leads to a disastrous-looking mess. Of course, even if I take the approach of adding/deleting atoms as needed their display still needs to be independent of the molecule display controls.
comment:6 by , 8 years ago
Hi Tristan, Those pins are totally cool! I just thought of the magic trick you need to avoid accidental display of your pins, the atom hide attribute. This is used to hide backbone atoms during ribbon display. So for all the pins you don’t want shown set atom.hide = True. I don’t think there is much code that unhides atoms except for ribbon drawing so hopefully the will not get accidentally displayed. I think pseudobonds to hidden atoms are shown, so you will also what to use bond.display = False for all the pseudobonds you want to hide. The atomspec_atoms() stuff is strictly for the command line parsing to decide if model “#5" has atoms (as opposed to be a surface or volume or some other model type). Overriding that is going to be very erratic and I just can’t see embedding atomspec_atoms() deep in key classes like Objects. The toolbar icon that is show atoms is not intended to apply only to “real atoms”. I have used fake atoms more than anyone else in Chimera and it is totally key that all the operations (except ones involving chemistry, like hydrogen bonds finding) behave in the same way as the do for real atoms. I realize you want a different flavor of fake atoms that we have never supported. If we had multiple use cases where we needed that we could look into adding some API for it, not hijacking some API used for command-line parsing that was not intended for that purpose. Tom
comment:8 by , 8 years ago
Did atom.hide = True solve your problem?
And did you get this trac comment in email? We are debugging why you are not getting email on trac tickets.
comment:9 by , 8 years ago
Did atom.hide = True solve your problem?
And did you get this trac comment in email? We are debugging why you are not getting email on trac tickets.
comment:10 by , 8 years ago
Yes to both, thanks! Tristan Croll Research Fellow Cambridge Institute for Medical Research University of Cambridge CB2 0XY
follow-up: 10 comment:11 by , 8 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Using atom.hide = True did the trick.
comment:12 by , 8 years ago
Sorry to resurrect this, but one more note: this doesn't play entirely
nicely with the show/hide ribbons functions. If I do:
show selAtoms ribbons
... then no ribbon is shown for the PositionRestraintIndicator model
(good, what I wanted - I never defined any bonds, and in addition I've
overridden ribbon_display_count to always return 0).
But when I then do:
hide selAtoms ribbons
... then all my indicators corresponding to backbone atoms have their
hide properties reset to False - regardless of the state of
ribbon_hide_backbone(s) for the residues in the
PositionRestraintIndicator model. I've worked around it by adding an
extra check under control of the atomic changes trigger:
def _update_target_display(self, trigger_name, changes):
if 'display changed' in changes.atom_reasons():
if self.master_model in
changes.modified_atoms().unique_structures:
self.target_indicators.displays = self.atoms.displays
if 'hide changed' in changes.atom_reasons():
if self._target_model in
changes.modified_atoms().unique_structures:
self.hidden_indicators.hides = True
... but would it be better if Residue.ribbon_hide_backbone = False meant
that changes to ribbon displays simply didn't affect the Atom.hide
property at all?
On 2017-05-26 17:52, ChimeraX wrote:
follow-up: 12 comment:13 by , 8 years ago
It seems reasonable that if
Residue.ribbon_hide_backbone = False
then the ribbon code should neither hide, nor unhide the residue atoms. This may cause problems though if the current mechanism to show the backbone atoms is to set this value to false with the expectation that ribbon drawing will then update and unhide the atoms. Perhaps if the unhiding only applied to the backbone atoms this would be good enough for your use.
Conrad implemented ribbons and is the expert on all things ribbons so he should decide.
I suggest you make a new ticket about ribbon hiding and assign it to Conrad since this ticket.
comment:14 by , 8 years ago
| Component: | Unassigned → Depiction |
|---|
The atomspec_atoms() method is used for parsing commands, for example "show #1@CA atoms" would use it to ask model #1 which atoms belongs to it. While you could have it return no atoms, as you have seen this is not going to disable a wide variety of operations on atoms (coloring, style changes, display changes) that are done without using a command with an atom specifier. The atomspec_atom() routine has a very narrow purpose and it would not make sense to change the very general object access code to use it.
I'm not sure exactly what behaviors you are trying to prevent on these fake atoms. If it is display, style and color changes then perhaps you want to replace the display, draw_mode and color property setter methods on each fake atom you create. This may not be the right solution -- I'd have to know the behaviors you are trying to achieve. Another idea is maybe you don't want to add the fake atoms model to the open model list (although that will mean those atoms are not drawn). One more suggestion: you should use the Structure class, not AtomicStructure, since AtomicStructure is for chemically sensible molecules.