add get_lattice_vectors, get_lattice_vector_angles and their respective tests

Author: cadenmyers13

src/diffpy/structure/structure.py

@@ -250,6 +250,31 @@ def get_occupancies(self):
         occupancies = numpy.array([a.occupancy for a in self])
         return occupancies
 
+    def get_lattice_vectors(self):
+        """Return array of lattice vectors for this structure.
+
+        Returns
+        -------
+        numpy.ndarray
+            The array of lattice vectors for this structure.
+        """
+        lattice_vectors = self.lattice.base
+        return lattice_vectors
+
+    def get_lattice_vector_angles(self):
+        """Return array of lattice vector angles for this structure.
+
+        Returns
+        -------
+        numpy.ndarray
+            The array of lattice vector angles for this structure.
+        """
+        a, b, c = self.lattice.base
+        alpha = self.lattice.angle(b, c)
+        beta = self.lattice.angle(a, c)
+        gamma = self.lattice.angle(a, b)
+        return numpy.array([alpha, beta, gamma])
+
     def convert_ase_to_diffpy_structure(
         self,
         ase_atoms: ASEAtoms,
@@ -269,8 +294,6 @@ def convert_ase_to_diffpy_structure(
 
         Returns
         -------
-        Structure
-            Reference to this `Structure` object with updated attributes and `Atom` instances.
         lost_info : dict, optional
             The dictionary containing any information from the ASE `Atoms`
             object that is not currently available in the `Structure` class.
@@ -317,31 +340,24 @@ def convert_ase_to_diffpy_structure(
 
         will return a dictionary with the magnetic moments of the atoms in the ASE `Atoms` object.
         """
+        # clear structure before populating it with new atoms
+        del self[:]
         if not isinstance(ase_atoms, ASEAtoms):
-            raise TypeError("Input must be an instance of ase.Atoms.")
-        # --- structure conversion ---
+            raise TypeError(f"Input must be an instance of ase.Atoms but got type {ase_atoms}.")
+        cell = ase_atoms.get_cell()
+        self.lattice = Lattice(base=numpy.array(cell))
         symbols = ase_atoms.get_chemical_symbols()
         scaled_positions = ase_atoms.get_scaled_positions()
         for sym, xyz in zip(symbols, scaled_positions):
             self.append(Atom(sym, xyz=xyz))
-        # --- optional extraction ---
         if lost_info is None:
             return
         extracted_info = {}
         for name in lost_info:
             if not hasattr(ase_atoms, name):
                 raise ValueError(f"ASE.Atoms object has no attribute '{name}'.")
-            try:
-                attr = getattr(ase_atoms, name)
-                value = attr() if callable(attr) else attr
-                # try to copy (safe for numpy arrays, dicts, etc.)
-                try:
-                    value = value.copy()
-                except Exception:
-                    pass
-                extracted_info[name] = value
-            except Exception as e:
-                extracted_info[name] = f"ERROR: {type(e).__name__}: {e}"
+            attr = getattr(ase_atoms, name)
+            extracted_info[name] = attr() if callable(attr) else attr
         return extracted_info
 
     def assign_unique_labels(self):

tests/conftest.py

@@ -1,7 +1,11 @@
 import json
 from pathlib import Path
 
+import numpy as np
 import pytest
+from ase import Atoms
+
+from diffpy.structure import Atom, Lattice, Structure
 
 
 @pytest.fixture
@@ -27,3 +31,45 @@ def _load(filename):
         return "tests/testdata/" + filename
 
     return _load
+
+
+@pytest.fixture
+def build_ase_atom_object():
+    """Helper function to build an ASE.Atoms object for testing."""
+    a = 5.409
+    frac_coords = np.array(
+        [
+            [0.0, 0.0, 0.0],
+            [0.5, 0.5, 0.5],
+            [0.25, 0.25, 0.25],
+            [0.75, 0.75, 0.75],
+        ]
+    )
+    cart_coords = frac_coords * a
+    symbols = ["Zn", "Zn", "S", "S"]
+    ase_zb = Atoms(symbols=symbols, positions=cart_coords, cell=[[a, 0, 0], [0, a, 0], [0, 0, a]], pbc=True)
+    return ase_zb
+
+
+@pytest.fixture
+def build_diffpy_structure_object():
+    """Helper function to build a diffpy.structure.Structure object for
+    testing."""
+    a = 5.409
+    frac_coords = np.array(
+        [
+            [0.0, 0.0, 0.0],
+            [0.5, 0.5, 0.5],
+            [0.25, 0.25, 0.25],
+            [0.75, 0.75, 0.75],
+        ]
+    )
+    lattice = Lattice(base=[[a, 0, 0], [0, a, 0], [0, 0, a]])
+    atoms = [
+        Atom("Zn", frac_coords[0]),
+        Atom("Zn", frac_coords[1]),
+        Atom("S", frac_coords[2]),
+        Atom("S", frac_coords[3]),
+    ]
+    diffpy_zb = Structure(atoms=atoms, lattice=lattice)
+    return diffpy_zb

tests/test_structure.py

@@ -686,9 +686,112 @@ def test_get_occupancies(datafile):
     assert numpy.allclose(actual_occupancies, expected_occupancies)
 
 
-# def test_convert_ase_to_diffpy_structure(datafile):
-#     """Check convert_ase_to_diffpy_structure()"""
-#     assert False
+def test_get_lattice_vectors(datafile):
+    """Check Structure.get_lattice_vectors()"""
+    pbte_stru = Structure(filename=datafile("PbTe.cif"))
+    actual_lattice_vectors = pbte_stru.get_lattice_vectors()
+    expected_lattice_vectors = numpy.array([[6.461, 0.0, 0.0], [0.0, 6.461, 0.0], [0.0, 0.0, 6.461]])
+    assert numpy.allclose(actual_lattice_vectors, expected_lattice_vectors)
+
+
+def test_get_lattice_vector_angles(datafile):
+    """Check Structure.get_lattice_vector_angles()"""
+    pbte_stru = Structure(filename=datafile("PbTe.cif"))
+    actual_lattice_vector_angles = pbte_stru.get_lattice_vector_angles()
+    expected_lattice_vector_angles = numpy.array([90.0, 90.0, 90.0])
+    assert numpy.allclose(actual_lattice_vector_angles, expected_lattice_vector_angles)
+
+
+@pytest.mark.parametrize(
+    "input",
+    [
+        # case: user calls the conversion function on a Structure object that already contains
+        # a structure
+        # expected: the structure is wiped clean and replaced with the converted structure
+        # we use the fixture to create a Structure object that already contains a structure.
+        "use_diffpy_structure_fixture",
+        # case: user calls the conversion function on an empty Structure object
+        # expected: the converted structure is added to the empty Structure object without issue
+        Structure(),
+    ],
+)
+def test_convert_ase_to_diffpy_structure(input, build_ase_atom_object, build_diffpy_structure_object):
+    """Check convert_ase_to_diffpy_structure()"""
+    # input: User wants to convert an ASE.Atoms object to a diffpy.structure.Structure object
+    # expected: All similar data is transferred correctly,
+    #           including chemical symbols, fractional coordinates, and lattice parameters.
+
+    # Create an ASE.Atoms object
+    ase_zb = build_ase_atom_object
+    # Create an identical expected diffpy Structure object
+    expected_structure = build_diffpy_structure_object
+
+    # Create new Structure object and convert ase to diffpy structure.
+    # Use the string input to determine which type of Structure object to create for the test
+    if isinstance(input, str):
+        actual_structure = build_diffpy_structure_object
+    else:
+        actual_structure = input
+    # set the lost_info variable, which gets the attribute of method from ASE.Atoms object, gets the values
+    # and stores it in a dict. This is used because ASE.Atoms stores more/different
+    # info that a diffpy.structure object
+    lost_info_dict = actual_structure.convert_ase_to_diffpy_structure(ase_zb, lost_info=["get_masses"])
+    actual_masses = lost_info_dict["get_masses"]
+    expected_masses = ase_zb.get_masses()
+    assert numpy.allclose(actual_masses, expected_masses)
+
+    # Compare the actual and expected values
+    expected_lattice_vectors = expected_structure.get_lattice_vectors()
+    actual_lattice_vectors = actual_structure.get_lattice_vectors()
+    assert numpy.allclose(expected_lattice_vectors, actual_lattice_vectors)
+
+    expected_lattice_angle = expected_structure.get_lattice_vector_angles()
+    actual_lattice_angle = actual_structure.get_lattice_vector_angles()
+    assert numpy.allclose(expected_lattice_angle, actual_lattice_angle)
+
+    expected_symbols = expected_structure.get_chemical_symbols()
+    actual_symbols = actual_structure.get_chemical_symbols()
+    assert actual_symbols == expected_symbols
+
+    expected_coords = expected_structure.get_fractional_coordinates()
+    actual_coords = actual_structure.get_fractional_coordinates()
+    assert numpy.allclose(actual_coords, expected_coords)
+
+
+def test_convert_ase_to_diffpy_structure_bad_typeerror():
+    """Check convert_ase_to_diffpy_structure() with bad input."""
+    bad_input = "string"  # pass a string instead of ase.Atoms
+    expected_error_msg = "Input must be an instance of ase.Atoms but got type str."
+    actual_structure = Structure()
+    with pytest.raises(TypeError, match=expected_error_msg):
+        actual_structure.convert_ase_to_diffpy_structure(bad_input)
+
+
+@pytest.mark.parametrize(
+    "bad_lost_info,error,expected_error_msg",
+    [  # case: User provides an ASE.Atoms object but requests lost_info that is not an attribute of ASE.Atoms
+        # expected: A ValueError is raised with a clear error message indicating the requested lost_info
+        #           attribute is invalid.
+        (["invalid_method"], ValueError, "ASE.Atoms object has no attribute 'invalid_method'"),
+        # case: User provides an ASE.Atoms object but requests lost_info that is an attribute of ASE.Atoms
+        # but has not been set yet.
+        # expected: The error message from ase is raised indicating the specific issue with the
+        #           requested lost_info attribute.
+        # We set the expected error message to None because this expectation is
+        # out of our control, but it is good to make sure that we are
+        # raising the error from ASE.
+        (["get_magnetic_moments"], RuntimeError, None),
+    ],
+)
+def test_convert_ase_to_diffpy_structure_bad_valueerror(
+    bad_lost_info, error, expected_error_msg, build_ase_atom_object
+):
+    """Check convert_ase_to_diffpy_structure() with bad lost_info."""
+    ase_zb = build_ase_atom_object
+    actual_structure = Structure()
+    with pytest.raises(error, match=expected_error_msg):
+        actual_structure.convert_ase_to_diffpy_structure(ase_zb, lost_info=bad_lost_info)
+
 
 # ----------------------------------------------------------------------------