Better design of `_StructureDataBaseViewer` and `StructureDataViewer`

[yakutovicha]

As a developer, one would like to have a clear separation of concern between _StructureDataBaseViewer and StructureDataViewer allowing for easy further improvements and modifications.

The base class is meant to provide a common functionality of structure viewer and (yet to come) trajectory viewer. Unfortunately, due to some design flows it cannot be easily done with the current implementation.

Herein, after a meeting with @unkcpz and @danielhollas, we agreed to have a closer look at those two classes and come up with a better design.

Known issues

1. Both structure and displayed_structure traits are defined in the StructureDataViewer class but used in _StructureDataBaseViewer.
2. Point 1. also makes it impossible to use _StructureDataBaseViewer as the base class for the trajectory viewer.
3. An addition of new displayed properties requires new traits to _StructureDataBaseViewer, which is not optimal as most of the features are stored in the structure trait.

Pinging @danielhollas and @unkcpz for additional input.

[yakutovicha]

Overview of the _StructureDataBaseViewer:

class _StructureDataBaseViewer(ipw.VBox):
    """Base viewer class for AiiDA structure or trajectory objects.

    :param configure_view: If True, add configuration tabs (deprecated)
    :type configure_view: bool
    :param configuration_tabs: List of configuration tabs (default: ["Selection", "Appearance", "Cell", "Download"])
    :type configure_view: list
    :param default_camera: default camera (orthographic|perspective), can be changed in the Appearance tab
    :type default_camera: string

    """

    selection = List(Int)
    selection_adv = Unicode()
    supercell = List(Int)
    cell = Instance(Cell, allow_none=True)
    DEFAULT_SELECTION_OPACITY = 0.2
    DEFAULT_SELECTION_RADIUS = 6
    DEFAULT_SELECTION_COLOR = "green"

    def __init__(self, ...):

    def _selection_tab(self):  # Defines a tab with the selection options.

    def _appearance_tab(self):  # Defines a tab which manages appearances.

    def _cell_tab(self):  # Defines a tab which displays cell info

    @observe("cell")
    def _observe_cell(self, _=None):  # Observes cell modifications and updates the displayed cell parameters.

    def _download_tab(self):  # Defines a tab with download options.

    def _render_structure(self, change=None):  # Function to render structure with povray.

    def _on_atom_click(self, _=None):  # Update selection when clicking on atom

    def highlight_atoms(self, ...) # Highlighting atoms according to the provided list.

    @observe("selection")
    def _observe_selection(self, _=None):  # Update selection string when selection is made

    def apply_selection(self, _=None):  # Apply selection from the selection string

    def download(self, change=None):  # Prepare structure for downloading 

    @staticmethod
    def _download(payload, filename):  # Use Javascript to download structure as a file.

    def _prepare_payload(self, file_format=None):  # Convert file to a string for downloading

    @property
    def thumbnail(self):  # Make a structure thumbnail. not sure if it is used

[yakutovicha]

Overview of the

@register_viewer_widget("data.cif.CifData.")
@register_viewer_widget("data.structure.StructureData.")
class StructureDataViewer(_StructureDataBaseViewer):
    """Viewer class for AiiDA structure objects.

    Attributes:
        structure (Atoms, StructureData, CifData): Trait that contains a structure object,
        which was initially provided to the viewer. It can be either directly set to an
        ASE Atoms object or to AiiDA structure object containing `get_ase()` method.

        displayed_structure (Atoms): Trait that contains a structure object that is
        currently displayed (super cell, for example). The trait is generated automatically
        and can't be set outside of the class.
    """

    structure = Union([Instance(Atoms), Instance(Node)], allow_none=True)
    displayed_structure = Instance(Atoms, allow_none=True, read_only=True)
    pk = Int(allow_none=True)

    def __init__(self, structure=None, **kwargs):
        # self.supercell.observe(self.repeat, names='value')

    @observe("supercell")
    def repeat(self, _=None):
        if self.structure is not None:
            self.set_trait("displayed_structure", self.structure.repeat(self.supercell))

    @validate("structure")
    def _valid_structure(self, change):  # pylint: disable=no-self-use

    @observe("structure")
    def _observe_structure(self, change):

    @observe("displayed_structure")
    def _update_structure_viewer(self, change):

    def d_from(self, operand):
        point = np.array([float(i) for i in operand[1:-1].split(",")])
        return np.linalg.norm(self.structure.positions - point, axis=1)

    def name_operator(self, operand):

    def not_operator(self, operand):
        )

    def parse_advanced_sel(self, condition=None):

    def create_selection_info(self):

    @observe("selection_adv")
    def _observe_selection_adv(self, _=None):

    @observe("selection")
    def _observe_selection_2(self, _=None):

[danielhollas]

@yakutovicha thanks for the overview! I can take this on in September, will not have time in the next two weeks.

Quick thoughts:

• If the _StructureDataBaseViewer is supposed the base class for both StructureDataViewer and TrajectoryDataViewer, then it should contain all common methods, which I suspect is most of them. e.g. some of the methods e.g. around selection should be moved from StructureDataViewer to base class.
• The base class is already pretty heavy. Perhaps instead we could split it into individual components, that would than be used in StructureDataViewer and TrajectoryDataViewer. One such component would be the AseStructureViewer, that would be solely concerned about displaying whats in the displayed_structure trait. The individual tabs could then be independent components as well. I'll try to prototype to see if this would work.