Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#6719 closed enhancement (fixed)

Block trigger handlers

Reported by: Tristan Croll Owned by: pett
Priority: normal Milestone:
Component: Core Version:
Keywords: Cc: chimera-programmers
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

The following bug report has been submitted:
Platform:        Linux-5.13.0-40-generic-x86_64-with-glibc2.31
ChimeraX Version: 1.3 (2021-12-08 23:08:33 UTC)
Description
I keep running into situations where it would be convenient to temporarily block a given trigger handler rather than the entire trigger (e.g. allowing the GUI to update settings and setting changes from the API to update the GUI without getting into an infinite loop). One way to achieve that is via a simple context manager to just remove and reinstate it:

{{{
from contextlib import contextmanager

@contextmanager
def block_trigger_handler(handler):
    name = handler._name
    func = handler._func
    triggerset = handler._trigger_set
    triggerset.remove_handler(handler)
    try:
        yield
    finally:
        triggerset.add_handler(name, func)
}}}

... but considering that this involves the use of three private variables it's not the best approach. Might be nice to give the `_TriggerHandler` class its own block method? 

OpenGL version: 3.3.0 NVIDIA 510.47.03
OpenGL renderer: NVIDIA TITAN Xp/PCIe/SSE2
OpenGL vendor: NVIDIA Corporation
Manufacturer: Dell Inc.
Model: Precision T5600
OS: Ubuntu 20.04 focal
Architecture: 64bit ELF
Virutal Machine: none
CPU: 32 Intel(R) Xeon(R) CPU E5-2687W 0 @ 3.10GHz
Cache Size: 20480 KB
Memory:
	              total        used        free      shared  buff/cache   available
	Mem:           62Gi        14Gi        37Gi       244Mi        10Gi        47Gi
	Swap:         4.9Gi          0B       4.9Gi

Graphics:
	03:00.0 VGA compatible controller [0300]: NVIDIA Corporation GP102 [TITAN Xp] [10de:1b02] (rev a1)	
	Subsystem: NVIDIA Corporation GP102 [TITAN Xp] [10de:11df]	
	Kernel driver in use: nvidia
Locale: ('en_GB', 'UTF-8')
PyQt5 5.15.2, Qt 5.15.2
Installed Packages:
    alabaster: 0.7.12
    appdirs: 1.4.4
    Babel: 2.9.1
    backcall: 0.2.0
    blockdiag: 2.0.1
    certifi: 2021.10.8
    cftime: 1.5.1.1
    charset-normalizer: 2.0.9
    ChimeraX-AddCharge: 1.2.2
    ChimeraX-AddH: 2.1.11
    ChimeraX-AlignmentAlgorithms: 2.0
    ChimeraX-AlignmentHdrs: 3.2
    ChimeraX-AlignmentMatrices: 2.0
    ChimeraX-Alignments: 2.2.3
    ChimeraX-AlphaFold: 1.0
    ChimeraX-AltlocExplorer: 1.0.1
    ChimeraX-AmberInfo: 1.0
    ChimeraX-Arrays: 1.0
    ChimeraX-Atomic: 1.31
    ChimeraX-AtomicLibrary: 4.2
    ChimeraX-AtomSearch: 2.0
    ChimeraX-AtomSearchLibrary: 1.0
    ChimeraX-AxesPlanes: 2.0
    ChimeraX-BasicActions: 1.1
    ChimeraX-BILD: 1.0
    ChimeraX-BlastProtein: 2.0
    ChimeraX-BondRot: 2.0
    ChimeraX-BugReporter: 1.0
    ChimeraX-BuildStructure: 2.6.1
    ChimeraX-Bumps: 1.0
    ChimeraX-BundleBuilder: 1.1
    ChimeraX-ButtonPanel: 1.0
    ChimeraX-CageBuilder: 1.0
    ChimeraX-CellPack: 1.0
    ChimeraX-Centroids: 1.2
    ChimeraX-ChemGroup: 2.0
    ChimeraX-Clashes: 2.2.2
    ChimeraX-Clipper: 0.17.0
    ChimeraX-ColorActions: 1.0
    ChimeraX-ColorGlobe: 1.0
    ChimeraX-ColorKey: 1.5
    ChimeraX-CommandLine: 1.1.5
    ChimeraX-ConnectStructure: 2.0
    ChimeraX-Contacts: 1.0
    ChimeraX-Core: 1.3
    ChimeraX-CoreFormats: 1.1
    ChimeraX-coulombic: 1.3.2
    ChimeraX-Crosslinks: 1.0
    ChimeraX-Crystal: 1.0
    ChimeraX-CrystalContacts: 1.0
    ChimeraX-DataFormats: 1.2.2
    ChimeraX-Dicom: 1.0
    ChimeraX-DistMonitor: 1.1.5
    ChimeraX-DistUI: 1.0
    ChimeraX-Dssp: 2.0
    ChimeraX-EMDB-SFF: 1.0
    ChimeraX-ExperimentalCommands: 1.0
    ChimeraX-FileHistory: 1.0
    ChimeraX-FunctionKey: 1.0
    ChimeraX-Geometry: 1.1
    ChimeraX-gltf: 1.0
    ChimeraX-Graphics: 1.1
    ChimeraX-Hbonds: 2.1.2
    ChimeraX-Help: 1.2
    ChimeraX-HKCage: 1.3
    ChimeraX-IHM: 1.1
    ChimeraX-ImageFormats: 1.2
    ChimeraX-IMOD: 1.0
    ChimeraX-IO: 1.0.1
    ChimeraX-ISOLDE: 1.3
    ChimeraX-ItemsInspection: 1.0
    ChimeraX-Label: 1.1
    ChimeraX-LinuxSupport: 1.0
    ChimeraX-ListInfo: 1.1.1
    ChimeraX-Log: 1.1.4
    ChimeraX-LookingGlass: 1.1
    ChimeraX-Maestro: 1.8.1
    ChimeraX-Map: 1.1
    ChimeraX-MapData: 2.0
    ChimeraX-MapEraser: 1.0
    ChimeraX-MapFilter: 2.0
    ChimeraX-MapFit: 2.0
    ChimeraX-MapSeries: 2.1
    ChimeraX-Markers: 1.0
    ChimeraX-Mask: 1.0
    ChimeraX-MatchMaker: 2.0.4
    ChimeraX-MDcrds: 2.6
    ChimeraX-MedicalToolbar: 1.0.1
    ChimeraX-Meeting: 1.0
    ChimeraX-MLP: 1.1
    ChimeraX-mmCIF: 2.4
    ChimeraX-MMTF: 2.1
    ChimeraX-Modeller: 1.2.6
    ChimeraX-ModelPanel: 1.2.1
    ChimeraX-ModelSeries: 1.0
    ChimeraX-Mol2: 2.0
    ChimeraX-Morph: 1.0
    ChimeraX-MouseModes: 1.1
    ChimeraX-Movie: 1.0
    ChimeraX-Neuron: 1.0
    ChimeraX-Nucleotides: 2.0.2
    ChimeraX-OpenCommand: 1.7
    ChimeraX-PDB: 2.6.5
    ChimeraX-PDBBio: 1.0
    ChimeraX-PDBLibrary: 1.0.2
    ChimeraX-PDBMatrices: 1.0
    ChimeraX-Phenix: 0.3
    ChimeraX-PickBlobs: 1.0
    ChimeraX-Positions: 1.0
    ChimeraX-PresetMgr: 1.0.1
    ChimeraX-PubChem: 2.1
    ChimeraX-ReadPbonds: 1.0.1
    ChimeraX-Registration: 1.1
    ChimeraX-RemoteControl: 1.0
    ChimeraX-ResidueFit: 1.0
    ChimeraX-RestServer: 1.1
    ChimeraX-RNALayout: 1.0
    ChimeraX-RotamerLibMgr: 2.0.1
    ChimeraX-RotamerLibsDunbrack: 2.0
    ChimeraX-RotamerLibsDynameomics: 2.0
    ChimeraX-RotamerLibsRichardson: 2.0
    ChimeraX-Sample: 0.1
    ChimeraX-SaveCommand: 1.5
    ChimeraX-SchemeMgr: 1.0
    ChimeraX-SDF: 2.0
    ChimeraX-Segger: 1.0
    ChimeraX-Segment: 1.0
    ChimeraX-SelInspector: 1.0
    ChimeraX-SeqView: 2.4.6
    ChimeraX-Shape: 1.0.1
    ChimeraX-Shell: 1.0
    ChimeraX-Shortcuts: 1.1
    ChimeraX-ShowAttr: 1.0
    ChimeraX-ShowSequences: 1.0
    ChimeraX-SideView: 1.0
    ChimeraX-Smiles: 2.1
    ChimeraX-SmoothLines: 1.0
    ChimeraX-SpaceNavigator: 1.0
    ChimeraX-StdCommands: 1.6.1
    ChimeraX-STL: 1.0
    ChimeraX-Storm: 1.0
    ChimeraX-Struts: 1.0
    ChimeraX-Surface: 1.0
    ChimeraX-SwapAA: 2.0
    ChimeraX-SwapRes: 2.1
    ChimeraX-TapeMeasure: 1.0
    ChimeraX-Test: 1.0
    ChimeraX-Toolbar: 1.1
    ChimeraX-ToolshedUtils: 1.2
    ChimeraX-Tug: 1.0
    ChimeraX-UI: 1.13.7
    ChimeraX-uniprot: 2.2
    ChimeraX-UnitCell: 1.0
    ChimeraX-ViewDockX: 1.0.1
    ChimeraX-VIPERdb: 1.0
    ChimeraX-Vive: 1.1
    ChimeraX-VolumeMenu: 1.0
    ChimeraX-Voyager: 0.1
    ChimeraX-VTK: 1.0
    ChimeraX-WavefrontOBJ: 1.0
    ChimeraX-WebCam: 1.0
    ChimeraX-WebServices: 1.0
    ChimeraX-Zone: 1.0
    colorama: 0.4.4
    cxservices: 1.1
    cycler: 0.11.0
    Cython: 0.29.24
    decorator: 5.1.0
    distro: 1.6.0
    docutils: 0.17.1
    filelock: 3.0.12
    funcparserlib: 0.3.6
    grako: 3.16.5
    h5py: 3.6.0
    html2text: 2020.1.16
    idna: 3.3
    ihm: 0.21
    imagecodecs: 2021.4.28
    imagesize: 1.3.0
    ipykernel: 5.5.5
    ipython: 7.23.1
    ipython-genutils: 0.2.0
    jedi: 0.18.0
    Jinja2: 3.0.1
    jsonpickle: 2.0.0
    jupyter-client: 6.1.12
    jupyter-core: 4.9.1
    kiwisolver: 1.3.2
    line-profiler: 3.3.0
    lxml: 4.6.3
    lz4: 3.1.3
    MarkupSafe: 2.0.1
    matplotlib: 3.4.3
    matplotlib-inline: 0.1.3
    msgpack: 1.0.2
    netCDF4: 1.5.7
    networkx: 2.6.2
    numexpr: 2.8.0
    numpy: 1.21.2
    openvr: 1.16.801
    packaging: 21.3
    ParmEd: 3.2.0
    parso: 0.8.3
    pexpect: 4.8.0
    pickleshare: 0.7.5
    Pillow: 8.3.2
    pip: 21.2.4
    pkginfo: 1.7.1
    prompt-toolkit: 3.0.23
    psutil: 5.8.0
    ptyprocess: 0.7.0
    pycollada: 0.7.1
    pydicom: 2.1.2
    Pygments: 2.10.0
    PyOpenGL: 3.1.5
    PyOpenGL-accelerate: 3.1.5
    pyparsing: 3.0.6
    PyQt5-commercial: 5.15.2
    PyQt5-sip: 12.8.1
    PyQtWebEngine-commercial: 5.15.2
    python-dateutil: 2.8.2
    python-igraph: 0.9.6
    pytz: 2021.3
    pyvis: 0.1.9
    pyzmq: 22.3.0
    qtconsole: 5.1.1
    QtPy: 1.11.3
    RandomWords: 0.3.0
    rdkit-pypi: 2021.3.5.1
    requests: 2.26.0
    scipy: 1.7.1
    setuptools: 57.5.0
    sfftk-rw: 0.7.1
    six: 1.16.0
    snowballstemmer: 2.2.0
    sortedcontainers: 2.4.0
    Sphinx: 4.2.0
    sphinx-autodoc-typehints: 1.12.0
    sphinxcontrib-applehelp: 1.0.2
    sphinxcontrib-blockdiag: 2.0.0
    sphinxcontrib-devhelp: 1.0.2
    sphinxcontrib-htmlhelp: 2.0.0
    sphinxcontrib-jsmath: 1.0.1
    sphinxcontrib-qthelp: 1.0.3
    sphinxcontrib-serializinghtml: 1.1.5
    suds-jurko: 0.6
    tables: 3.6.1
    texttable: 1.6.4
    tifffile: 2021.4.8
    tinyarray: 1.2.3
    tornado: 6.1
    traitlets: 5.1.1
    urllib3: 1.26.7
    wcwidth: 0.2.5
    webcolors: 1.11.1
    wheel: 0.37.0
    wheel-filename: 1.3.0

Change History (5)

comment:1 by pett, 3 years ago

Cc: chimera-programmers added
Component: UnassignedCore
Owner: set to pett
Platform: all
Project: ChimeraX
Status: newaccepted
Summary: ChimeraX bug report submissionBlock trigger handlers
Type: defectenhancement

Seems okay to me. Will leave this open for a few days to give the other programmers a chance to weigh in if desired.

comment:2 by Tom Goddard, 3 years ago

Seems ok.

comment:3 by Tristan Croll, 3 years ago

While debugging #8253 I found myself revisiting this... my existing hacky band-aid solution was a context manager to remove the existing handler and replace it when done:

@contextmanager
def block_managed_trigger_handler(object, handler_name):
    '''
    `object.handler_name` should be a ChimeraX trigger handler. The original handler 
    will be deleted, and replaced with an identical new one on exit of the context.
    '''
    h = getattr(object, handler_name)
    name = h._name
    func = h._func
    triggerset = h._trigger_set
    triggerset.remove_handler(h)
    try:
        yield
    finally:
        setattr(object, handler_name, triggerset.add_handler(name, func))

... but this was doing weird things in the context of #8253 (I guess the long delays were breaking some of PyQt's assumptions with regard to threading... eventually it would get to a state where things seemed to happen out of order - the old handler wasn't properly removed and fired when it shouldn't). If I instead do:

from chimerax.core.triggerset import _TriggerHandler
class BlockableTriggerHandler(_TriggerHandler):
    def invoke(self, data, remove_if_error):
        if getattr(self, '_blocked', 0):
            return
        super().invoke(data, remove_if_error)
    
    @contextmanager
    def block(self):
        self._blocked = getattr(self, '_blocked', 0) + 1
        try:
            yield
        finally:
            self._blocked -= 1
    
    def manual_block(self):
        self._blocked = getattr(self, '_blocked', 0) +1
    
    def manual_release(self):
        if getattr(self, '_blocked', 0) <= 0:
            raise RuntimeError('More releases than blocks')
        self._blocked -= 1

h = some_triggerset.add_handler('trigger name', some_func)
h.__class__ = BlockableTriggerHandler

with h.block():
    do_something()

... and everything is well-behaved... but I'd be much more comfortable without this sort of monkey-patching.

comment:4 by pett, 3 years ago

Resolution: fixed
Status: acceptedclosed

Okay, incorporated this but named the context manager "blocked" and dropped the "manual_" prefixes from block and release. Added an analogous context manager to the _Trigger class.

in reply to:  5 ; comment:5 by Tristan Croll, 3 years ago

Awesome, thanks!

On Tue, Jan 3, 2023 at 9:47 PM ChimeraX <ChimeraX-bugs-admin@cgl.ucsf.edu>
wrote:

Note: See TracTickets for help on using tickets.