From 08b4a047e0c9c51e6697630eb3d4892913ae6b62 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Tue, 3 Oct 2023 22:14:45 +0000 Subject: [PATCH 1/3] Not create cell for 0D `_validate_and_fix_ase_cell` function is not needed anymore, since it add a dummy cell to gas phase molecule. In the old AiiDA, there needed to be default cell, otherwise StructureData would not work. But that has been fixed so we should get rid of this login in AWB. The fix in aiida-core was done here aiidateam/aiida-core#5341 and released in version 2.0 https://github.com/aiidateam/aiida-core/blob/main/CHANGELOG.md#v200---2022-04-27 --- aiidalab_widgets_base/structures.py | 33 +++-------------------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/aiidalab_widgets_base/structures.py b/aiidalab_widgets_base/structures.py index e5f5100b1..6f2fc32d4 100644 --- a/aiidalab_widgets_base/structures.py +++ b/aiidalab_widgets_base/structures.py @@ -376,32 +376,6 @@ def __init__( children=[self.file_upload, supported_formats, self._status_message] ) - def _validate_and_fix_ase_cell(self, ase_structure, vacuum_ang=10.0): - """ - Checks if the ase Atoms object has a cell set, - otherwise sets it to bounding box plus specified "vacuum" space - """ - if not ase_structure: - return None - - cell = ase_structure.cell - - # TODO: Since AiiDA 2.0, zero cell is possible if PBC=false - # so we should honor that here and do not put artificial cell - # around gas phase molecules. - if ( - np.linalg.norm(cell[0]) < 0.1 - or np.linalg.norm(cell[1]) < 0.1 - or np.linalg.norm(cell[2]) < 0.1 - ): - # if any of the cell vectors is too short, consider it faulty - # set cell as bounding box + vacuum_ang - bbox = np.ptp(ase_structure.positions, axis=0) - new_structure = ase_structure.copy() - new_structure.cell = bbox + vacuum_ang - return new_structure - return ase_structure - def _on_file_upload(self, change=None): """When file upload button is pressed.""" for fname, item in change["new"].items(): @@ -444,9 +418,9 @@ def _read_structure(self, fname, content): return TrajectoryData( structurelist=[ StructureData( - ase=self._validate_and_fix_ase_cell(ase_struct) + ase=ase_structure, ) - for ase_struct in structures + for ase_structure in structures ] ) else: @@ -455,7 +429,7 @@ def _read_structure(self, fname, content): """ return None - return self._validate_and_fix_ase_cell(structures[0]) + return structures[0] class StructureExamplesWidget(ipw.VBox): @@ -720,7 +694,6 @@ def _make_ase(self, species, positions, smiles): if len(species) > 2: positions = PCA(n_components=3).fit_transform(positions) atoms = ase.Atoms(species, positions=positions, pbc=False) - atoms.cell = np.ptp(atoms.positions, axis=0) + 10 atoms.center() # We're attaching this info so that it # can be later stored as an extra on AiiDA Structure node. From c6bf1810e3d0f57a0bcdad297397d01b1a3d6bb8 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Tue, 3 Oct 2023 22:45:03 +0000 Subject: [PATCH 2/3] Refactoring: moving configuration_tabs to the create_tab function --- aiidalab_widgets_base/viewers.py | 56 +++++++++++++++++++------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/aiidalab_widgets_base/viewers.py b/aiidalab_widgets_base/viewers.py index d1ed774f9..a9d370479 100644 --- a/aiidalab_widgets_base/viewers.py +++ b/aiidalab_widgets_base/viewers.py @@ -171,45 +171,57 @@ def __init__( view_box = ipw.VBox([self._viewer]) - configuration_tabs_map = { - "Cell": self._cell_tab(), - "Selection": self._selection_tab(), - "Appearance": self._appearance_tab(), - "Download": self._download_tab(), - } + # Constructing configuration box + if configuration_tabs is None: + self._configuration_tabs = ["Selection", "Appearance", "Cell", "Download"] + else: + self._configuration_tabs = configuration_tabs if configure_view is not True: warnings.warn( - "`configure_view` is deprecated, please use `configuration_tabs` instead.", + "`configure_view` is deprecated, please use `configuration_tabs` instead. Will be removed in the version 3.0.0", DeprecationWarning, stacklevel=2, ) if not configure_view: - configuration_tabs.clear() + self._configuration_tabs.clear() - # Constructing configuration box - if configuration_tabs is None: - configuration_tabs = ["Selection", "Appearance", "Cell", "Download"] - if len(configuration_tabs) != 0: - self.configuration_box = ipw.Tab( - layout=ipw.Layout(flex="1 1 auto", width="auto") - ) - self.configuration_box.children = [ - configuration_tabs_map[tab_title] for tab_title in configuration_tabs - ] - - for i, title in enumerate(configuration_tabs): - self.configuration_box.set_title(i, title) - children = [ipw.HBox([view_box, self.configuration_box])] + if len(self._configuration_tabs) != 0: + configuration_box = self._create_tabs(self._configuration_tabs) + children = [ipw.HBox([view_box, configuration_box])] view_box.layout = {"width": "60%"} else: children = [view_box] if "children" in kwargs: + warnings.warn( + "defining `children` is deprecated, will be removed in the version 3.0.0", + DeprecationWarning, + stacklevel=2, + ) children += kwargs.pop("children") super().__init__(children, **kwargs) + def _create_tabs(self, configuration_tabs): + """Create tabs for configuration box.""" + configuration_tabs_map = { + "Cell": self._cell_tab(), + "Selection": self._selection_tab(), + "Appearance": self._appearance_tab(), + "Download": self._download_tab(), + } + + configuration_box = ipw.Tab(layout=ipw.Layout(flex="1 1 auto", width="auto")) + configuration_box.children = [ + configuration_tabs_map[tab_title] for tab_title in configuration_tabs + ] + + for i, title in enumerate(self._configuration_tabs): + configuration_box.set_title(i, title) + + return configuration_box + def _selection_tab(self): """Defining the selection tab.""" From 50e313d4f1d34d666d32af04fd531088513777cc Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Wed, 4 Oct 2023 00:07:40 +0000 Subject: [PATCH 3/3] The lattice tab will be hidden --- aiidalab_widgets_base/viewers.py | 71 ++++++++++++++++++++------------ tests/conftest.py | 8 ++++ tests/test_viewers.py | 6 +++ 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/aiidalab_widgets_base/viewers.py b/aiidalab_widgets_base/viewers.py index a9d370479..42e805fc3 100644 --- a/aiidalab_widgets_base/viewers.py +++ b/aiidalab_widgets_base/viewers.py @@ -169,14 +169,29 @@ def __init__( self._viewer.observe(self._on_atom_click, names="picked") self._viewer.stage.set_parameters(mouse_preset="pymol") - view_box = ipw.VBox([self._viewer]) + self.view_box = ipw.VBox([self._viewer]) # Constructing configuration box if configuration_tabs is None: - self._configuration_tabs = ["Selection", "Appearance", "Cell", "Download"] + self._configuration_tabs = [ + "Selection", + "Appearance", + "Lattice", + "Download", + ] else: self._configuration_tabs = configuration_tabs + # For backward compatibility, where the Lattice tab was called Cell. + if "Cell" in self._configuration_tabs: + warnings.warn( + "`Cell` tab is deprecated, please use `Lattice` instead. Will be removed in the version 3.0.0", + DeprecationWarning, + stacklevel=2, + ) + self._configuration_tabs.remove("Cell") + self._configuration_tabs.append("Lattice") + if configure_view is not True: warnings.warn( "`configure_view` is deprecated, please use `configuration_tabs` instead. Will be removed in the version 3.0.0", @@ -188,10 +203,10 @@ def __init__( if len(self._configuration_tabs) != 0: configuration_box = self._create_tabs(self._configuration_tabs) - children = [ipw.HBox([view_box, configuration_box])] - view_box.layout = {"width": "60%"} + children = [ipw.HBox([self.view_box, configuration_box])] + self.view_box.layout = {"width": "60%"} else: - children = [view_box] + children = [self.view_box] if "children" in kwargs: warnings.warn( @@ -206,7 +221,7 @@ def __init__( def _create_tabs(self, configuration_tabs): """Create tabs for configuration box.""" configuration_tabs_map = { - "Cell": self._cell_tab(), + "Lattice": self._cell_tab(), "Selection": self._selection_tab(), "Appearance": self._appearance_tab(), "Download": self._download_tab(), @@ -329,7 +344,24 @@ def change_camera(change): @tl.observe("cell") def _observe_cell(self, _=None): - # Updtate the Cell and Periodicity. + # Update the widget layout view. + if not self.cell and "Lattice" in self._configuration_tabs: + self._configuration_tabs.remove("Lattice") + + if self.cell and "Lattice" not in self._configuration_tabs: + self._configuration_tabs = ["Lattice"] + self._configuration_tabs + + if len(self._configuration_tabs) != 0: + configuration_box = self._create_tabs(self._configuration_tabs) + self.children = [ipw.HBox([self.view_box, configuration_box])] + self.view_box.layout = {"width": "60%"} + else: + self.children = [self.view_box] + + # Updtate the Lattice tab and Periodicity. + + # For molecules, the cell is Cell([0, 0, 0]) and in the condition is still # `False``. That's why it is able to use `if self.cell` to cover both None and + # not cell defined cases. if self.cell: self.cell_a.value = "a: {:.4f} {:.4f} {:.4f}".format( *self.cell.array[0] @@ -375,22 +407,6 @@ def _observe_cell(self, _=None): self.periodicity.value = ( f"Periodicity: {periodicity_map[tuple(self.structure.pbc)]}" ) - else: - self.cell_a.value = "a:" - self.cell_b.value = "b:" - self.cell_c.value = "c:" - - self.cell_a_length.value = "|a|:" - self.cell_b_length.value = "|b|:" - self.cell_c_length.value = "|c|:" - - self.cell_alpha.value = "α:" - self.cell_beta.value = "β:" - self.cell_gamma.value = "γ:" - - self.cell_spacegroup.value = "" - self.cell_hall.value = "" - self.periodicity.value = "" def _cell_tab(self): self.cell_a = ipw.HTML() @@ -409,8 +425,6 @@ def _cell_tab(self): self.cell_hall = ipw.HTML() self.periodicity = ipw.HTML() - self._observe_cell() - return ipw.VBox( [ ipw.HTML("Length unit: angstrom (Å)"), @@ -801,7 +815,9 @@ def __init__(self, structure=None, **kwargs): @tl.observe("supercell") def repeat(self, _=None): - if self.structure is not None: + # need to check if cell is defined, otherwise it will throw an error + # since it makes no sense to repeat a molecule + if self.structure is not None and self.cell: self.set_trait("displayed_structure", self.structure.repeat(self.supercell)) @tl.validate("structure") @@ -1108,7 +1124,8 @@ def add_info(indx, atom): @tl.observe("displayed_selection") def _observe_displayed_selection_2(self, _=None): - self.selection_info.value = self.create_selection_info() + if self.displayed_selection: + self.selection_info.value = self.create_selection_info() @register_viewer_widget("data.core.folder.FolderData.") diff --git a/tests/conftest.py b/tests/conftest.py index 9844bd207..28632789f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -203,6 +203,14 @@ def structure_data_object(): return structure +@pytest.fixture +def molecule_ase_object(): + """Return an ase molecule object.""" + from ase.build import molecule + + return molecule("H2O") + + @pytest.fixture def bands_data_object(): BandsData = plugins.DataFactory("core.array.bands") # noqa: N806 diff --git a/tests/test_viewers.py b/tests/test_viewers.py index b5c9287db..20ac2263a 100644 --- a/tests/test_viewers.py +++ b/tests/test_viewers.py @@ -104,3 +104,9 @@ def test_structure_data_viwer(structure_data_object): for fmt, out in format_cases: v.file_format.label = fmt assert v._prepare_payload() == out + + +def test_structure_viewer_for_molecule(molecule_ase_object): + """Test the structure viewer widget for a molecule.""" + v = viewers.StructureDataViewer(molecule_ase_object) + assert isinstance(v, viewers.StructureDataViewer)