Opened 6 years ago

Closed 6 years ago

#2940 closed defect (fixed)

subclassable AtomicStructure

Reported by: Ben Webb Owned by: Eric Pettersen
Priority: normal Milestone:
Component: Core Version:
Keywords: Cc: Tom Goddard
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

The following bug report has been submitted:
Platform:        Darwin-19.3.0-x86_64-i386-64bit
ChimeraX Version: 0.93 (2020-03-08)
Description
It is difficult to support sessions with objects that inherit from chimerax.atomic.AtomicStructure.

Several ChimeraX classes (e.g. Model, Structure) provide a set_state_from_snapshot() method that makes adding session support for derived classes easy, e.g.

class MyModel(Model):
    @staticmethod
    def restore_snapshot(session, data):
        m = MyModel(session)
        m.set_state_from_snapshot(data)
        return m

    def set_state_from_snapshot(self, session, data):
        Model.set_state_from_snapshot(self, session, data['model state'])
        # do something for MyModel with the rest of data 	

However, AtomicStructure does not follow this pattern. Thus, to provide a restore_snapshot() method for an AtomicStructure subclass, I have to duplicate some of the logic from AtomicStructure.restore_snapshot() (I cannot just call it from my derived class because it creates an AtomicStructure object, not a derived object). (A concrete example is the _RMFState class in the RMF plugin in the ChimeraX toolshed; see https://github.com/salilab/rmf_chimerax/blob/897d2f758/src/io.py#L65-L84 .) This is not ideal because it makes my code strongly dependent on the internals of AtomicStructure.

Could an AtomicStructure.set_state_from_snapshot() method be provided to fix this issue?

OpenGL version: 4.1 ATI-3.5.5
OpenGL renderer: AMD Radeon Pro 560 OpenGL Engine
OpenGL vendor: ATI Technologies Inc.

Change History (2)

comment:1 by Eric Pettersen, 6 years ago

Cc: Tom Goddard added
Component: UnassignedCore
Owner: set to Eric Pettersen
Platform: all
Project: ChimeraX
Status: newaccepted
Summary: ChimeraX bug report submissionsubclassable AtomicStructure

comment:2 by Eric Pettersen, 6 years ago

Resolution: fixed
Status: acceptedclosed

Hi Ben,

I have added the set_state_from_snapshot support to AtomicStructure.
It would be mildly better if real atomic structures were instances of AtomicStructure and pseudo-atomic structures were instances of Structure, since that's what those classes are used for, but I understand that that may not be easy in the data paradigm you're dealing with. Having pseudo-atomic structures as instances of AtomicStructure means, for example, that they will appear in lists of models for tools that deal with atomic structures (e.g. AddH) that may not work well with atoms that aren't in fact atoms, and that some algorithms may get applied to them (e.g. atom-type detection) that also might be slow when dealing with non-physical atomic connectivity.
This may not in fact be a big deal, but just giving you a heads up.

--Eric

Note: See TracTickets for help on using tickets.