Fix issues with labels (#3169)

* Add label support to (Periodic)Neighbor

* Default label to `species_string`

* allow empty string as label

* test None, '' and tuple Site labels

* optimade url use {response_fields=!s}

---------

Co-authored-by: Janosh Riebesell <[email protected]>

Author: stefsmeets

pymatgen/core/sites.py

@@ -71,7 +71,7 @@ def __init__(
         self._species: Composition = species  # type: ignore
         self.coords: np.ndarray = coords  # type: ignore
         self.properties: dict = properties or {}
-        self.label = label
+        self.label = label if label else self.species_string
 
     def __getattr__(self, attr):
         # overriding getattr doesn't play nicely with pickle, so we can't use self._properties
@@ -213,7 +213,7 @@ def __contains__(self, el):
     def __repr__(self):
         name = self.species_string
 
-        if self.label:
+        if self.label != name:
             name = f"{self.label} ({name})"
 
         return f"Site: {name} ({self.coords[0]:.4f}, {self.coords[1]:.4f}, {self.coords[2]:.4f})"
@@ -256,6 +256,9 @@ def as_dict(self) -> dict:
         }
         if self.properties:
             dct["properties"] = self.properties
+
+        dct["label"] = self.label
+
         return dct
 
     @classmethod
@@ -274,7 +277,8 @@ def from_dict(cls, dct: dict) -> Site:
         if props is not None:
             for key in props:
                 props[key] = json.loads(json.dumps(props[key], cls=MontyEncoder), cls=MontyDecoder)
-        return cls(atoms_n_occu, dct["xyz"], properties=props)
+        label = dct.get("label")
+        return cls(atoms_n_occu, dct["xyz"], properties=props, label=label)
 
 
 class PeriodicSite(Site, MSONable):
@@ -341,7 +345,7 @@ def __init__(
         self._species: Composition = species  # type: ignore
         self._coords: np.ndarray | None = None
         self.properties: dict = properties or {}
-        self.label = label
+        self.label = label if label else self.species_string
 
     def __hash__(self) -> int:
         """
@@ -551,7 +555,7 @@ def distance(self, other: PeriodicSite, jimage: ArrayLike | None = None):
     def __repr__(self):
         name = self.species_string
 
-        if self.label:
+        if self.label != name:
             name = f"{self.label} ({name})"
 
         x, y, z = self.coords
@@ -585,7 +589,6 @@ def as_dict(self, verbosity: int = 0) -> dict:
 
         if verbosity > 0:
             dct["xyz"] = [float(c) for c in self.coords]
-            dct["label"] = self.species_string
 
         dct["properties"] = self.properties
         dct["label"] = self.label

pymatgen/core/structure.py

@@ -69,19 +69,22 @@ def __init__(
         properties: dict | None = None,
         nn_distance: float = 0.0,
         index: int = 0,
+        label: str | None = None,
     ):
         """
         :param species: Same as Site
         :param coords: Same as Site, but must be fractional.
         :param properties: Same as Site
         :param nn_distance: Distance to some other Site.
         :param index: Index within structure.
+        :param label: Label for the site. Defaults to None.
         """
         self.coords = coords
         self._species = species
         self.properties = properties or {}
         self.nn_distance = nn_distance
         self.index = index
+        self.label = label if label is not None else self.species_string
 
     def __len__(self) -> Literal[3]:
         """Make neighbor Tuple-like to retain backwards compatibility."""
@@ -134,6 +137,7 @@ def __init__(
         nn_distance: float = 0.0,
         index: int = 0,
         image: tuple = (0, 0, 0),
+        label: str | None = None,
     ):
         """
         Args:
@@ -144,6 +148,7 @@ def __init__(
             nn_distance (float, optional): Distance to some other Site.. Defaults to 0.0.
             index (int, optional): Index within structure.. Defaults to 0.
             image (tuple, optional): PeriodicImage. Defaults to (0, 0, 0).
+            label (str, optional): Label for the site. Defaults to None.
         """
         self._lattice = lattice
         self._frac_coords = coords
@@ -152,6 +157,7 @@ def __init__(
         self.nn_distance = nn_distance
         self.index = index
         self.image = image
+        self.label = label if label is not None else self.species_string
 
     @property  # type: ignore
     def coords(self) -> np.ndarray:  # type: ignore
@@ -310,7 +316,7 @@ def site_properties(self) -> dict[str, Sequence]:
         return props
 
     @property
-    def labels(self) -> list[str | None]:
+    def labels(self) -> list[str]:
         """Return site labels as a list."""
         return [site.label for site in self]
 
@@ -2874,7 +2880,7 @@ def __init__(
         spin_multiplicity: int | None = None,
         validate_proximity: bool = False,
         site_properties: dict | None = None,
-        labels: list[str | None] | None = None,
+        labels: Sequence[str | None] | None = None,
         charge_spin_check: bool = True,
     ) -> None:
         """
@@ -4292,7 +4298,7 @@ def __init__(
         spin_multiplicity: int | None = None,
         validate_proximity: bool = False,
         site_properties: dict | None = None,
-        labels: list[str | None] | None = None,
+        labels: Sequence[str | None] | None = None,
         charge_spin_check: bool = True,
     ) -> None:
         """

pymatgen/core/tests/test_sites.py

@@ -36,33 +36,34 @@ def test_properties(self):
         assert self.propertied_site.properties["charge"] == 4.2
 
     def test_to_from_dict(self):
-        d = self.disordered_site.as_dict()
-        site = Site.from_dict(d)
+        dct = self.disordered_site.as_dict()
+        site = Site.from_dict(dct)
         assert site == self.disordered_site
         assert site != self.ordered_site
-        d = self.propertied_site.as_dict()
-        site = Site.from_dict(d)
+        dct = self.propertied_site.as_dict()
+        site = Site.from_dict(dct)
         assert site.properties["magmom"] == 5.1
         assert site.properties["charge"] == 4.2
-        d = self.propertied_magmom_vec_site.as_dict()
-        site = Site.from_dict(d)
+        dct = self.propertied_magmom_vec_site.as_dict()
+        site = Site.from_dict(dct)
         assert site.properties["magmom"] == Magmom([2.6, 2.6, 3.5])
         assert site.properties["charge"] == 4.2
-        d = self.dummy_site.as_dict()
-        site = Site.from_dict(d)
+        dct = self.dummy_site.as_dict()
+        site = Site.from_dict(dct)
         assert site.species == self.dummy_site.species
 
     def test_hash(self):
         assert hash(self.ordered_site) == 26
         assert hash(self.disordered_site) == 51
 
-    def test_cmp(self):
+    def test_gt_lt(self):
         assert self.ordered_site > self.disordered_site
+        assert self.disordered_site < self.ordered_site
 
     def test_distance(self):
-        osite = self.ordered_site
-        assert np.linalg.norm([0.25, 0.35, 0.45]) == osite.distance_from_point([0, 0, 0])
-        assert osite.distance(self.disordered_site) == 0
+        ord_site = self.ordered_site
+        assert np.linalg.norm([0.25, 0.35, 0.45]) == ord_site.distance_from_point([0, 0, 0])
+        assert ord_site.distance(self.disordered_site) == 0
 
     def test_pickle(self):
         o = pickle.dumps(self.propertied_site)
@@ -105,7 +106,7 @@ def test_properties(self):
         assert self.site.y == 3.5
         assert self.site.z == 4.5
         assert self.site.is_ordered
-        assert self.site.label is None
+        assert self.site.label == "Fe"
         assert not self.site2.is_ordered
         assert self.propertied_site.properties["magmom"] == 5.1
         assert self.propertied_site.properties["charge"] == 4.2
@@ -170,20 +171,23 @@ def test_equality_with_label(self):
         assert self.labeled_site == site
 
     def test_as_from_dict(self):
-        d = self.site2.as_dict()
-        site = PeriodicSite.from_dict(d)
+        dct = self.site2.as_dict()
+        site = PeriodicSite.from_dict(dct)
         assert site == self.site2
         assert site != self.site
-        assert site.label == self.site.label
-        d = self.propertied_site.as_dict()
+        assert site.label == self.site2.label
+
+        dct = self.propertied_site.as_dict()
         site3 = PeriodicSite({"Si": 0.5, "Fe": 0.5}, [0, 0, 0], self.lattice)
-        d = site3.as_dict()
-        site = PeriodicSite.from_dict(d)
+        dct = site3.as_dict()
+        site = PeriodicSite.from_dict(dct)
         assert site.species == site3.species
+        assert site.label == site3.label
 
-        d = self.dummy_site.as_dict()
-        site = PeriodicSite.from_dict(d)
+        dct = self.dummy_site.as_dict()
+        site = PeriodicSite.from_dict(dct)
         assert site.species == self.dummy_site.species
+        assert site.label == self.dummy_site.label
 
     def test_to_unit_cell(self):
         site = PeriodicSite("Fe", np.array([1.25, 2.35, 4.46]), self.lattice)

pymatgen/core/tests/test_structure.py

@@ -19,7 +19,15 @@
 from pymatgen.core.lattice import Lattice
 from pymatgen.core.operations import SymmOp
 from pymatgen.core.periodic_table import Element, Species
-from pymatgen.core.structure import IMolecule, IStructure, Molecule, PeriodicNeighbor, Structure, StructureError
+from pymatgen.core.structure import (
+    IMolecule,
+    IStructure,
+    Molecule,
+    Neighbor,
+    PeriodicNeighbor,
+    Structure,
+    StructureError,
+)
 from pymatgen.electronic_structure.core import Magmom
 from pymatgen.io.ase import AseAtomsAdaptor
 from pymatgen.util.testing import PymatgenTest
@@ -45,16 +53,21 @@ def test_msonable(self):
         nn = json.loads(str_, cls=MontyDecoder)
         assert isinstance(nn[0], PeriodicNeighbor)
 
+    def test_neighbor_labels(self):
+        comp = Composition("C")
+        for label in (None, "", "str label", ("tuple", "label")):
+            neighbor = Neighbor(comp, (0, 0, 0), label=label)
+            assert neighbor.label == label if label is not None else str(comp)
+
+            p_neighbor = PeriodicNeighbor(comp, (0, 0, 0), (10, 10, 10), label=label)
+            assert p_neighbor.label == label if label is not None else str(comp)
+
 
 class IStructureTest(PymatgenTest):
     def setUp(self):
         coords = [[0, 0, 0], [0.75, 0.5, 0.75]]
         self.lattice = Lattice(
-            [
-                [3.8401979337, 0.00, 0.00],
-                [1.9200989668, 3.3257101909, 0.00],
-                [0.00, -2.2171384943, 3.1355090603],
-            ]
+            [[3.8401979337, 0, 0], [1.9200989668, 3.3257101909, 0], [0, -2.2171384943, 3.1355090603]]
         )
         self.struct = IStructure(self.lattice, ["Si"] * 2, coords)
         assert len(self.struct) == 2, "Wrong number of sites in structure!"
@@ -69,11 +82,7 @@ def setUp(self):
         self.labeled_structure = IStructure(self.lattice, ["Si"] * 2, coords, labels=["Si1", "Si2"])
 
         self.lattice_pbc = Lattice(
-            [
-                [3.8401979337, 0.00, 0.00],
-                [1.9200989668, 3.3257101909, 0.00],
-                [0.00, -2.2171384943, 3.1355090603],
-            ],
+            [[3.8401979337, 0, 0], [1.9200989668, 3.3257101909, 0], [0, -2.2171384943, 3.1355090603]],
             pbc=(True, True, False),
         )
 
@@ -171,7 +180,7 @@ def test_fractional_occupations(self):
 
     def test_labeled_structure(self):
         assert self.labeled_structure.labels == ["Si1", "Si2"]
-        assert self.struct.labels == [None, None]
+        assert self.struct.labels == ["Si", "Si"]
 
     def test_get_distance(self):
         assert self.struct.get_distance(0, 1) == approx(2.35, abs=1e-2), "Distance calculated wrongly!"
@@ -318,7 +327,7 @@ def test_interpolate(self):
             assert interpolated_structs[0].lattice == inter_struct.lattice
         assert_array_equal(interpolated_structs[1][1].frac_coords, [0.625, 0.5, 0.625])
 
-        bad_lattice = [[1, 0.00, 0.00], [0, 1, 0.00], [0.00, 0, 1]]
+        bad_lattice = [[1, 0, 0], [0, 1, 0], [0, 0, 1]]
         struct2 = IStructure(bad_lattice, ["Si"] * 2, coords2)
         with pytest.raises(ValueError, match="Structures with different lattices"):
             struct.interpolate(struct2)
@@ -764,9 +773,7 @@ def setUp(self):
         coords = []
         coords.append([0, 0, 0])
         coords.append([0.75, 0.5, 0.75])
-        lattice = Lattice(
-            [[3.8401979337, 0.00, 0.00], [1.9200989668, 3.3257101909, 0.00], [0.00, -2.2171384943, 3.1355090603]]
-        )
+        lattice = Lattice([[3.8401979337, 0, 0], [1.9200989668, 3.3257101909, 0], [0, -2.2171384943, 3.1355090603]])
         self.structure = Structure(lattice, ["Si", "Si"], coords)
         self.cu_structure = Structure(lattice, ["Cu", "Cu"], coords)
         self.disordered = Structure.from_spacegroup("Im-3m", Lattice.cubic(3), [Composition("Fe0.5Mn0.5")], [[0, 0, 0]])
@@ -1157,7 +1164,7 @@ def test_from_magnetic_spacegroup(self):
             "P4_2'/mnm'",
             Lattice.tetragonal(4.87, 3.30),
             ["Mn", "F"],
-            [[0, 0, 0], [0.30, 0.30, 0.00]],
+            [[0, 0, 0], [0.30, 0.30, 0]],
             {"magmom": [4, 0]},
         )
 
@@ -1173,7 +1180,7 @@ def test_from_magnetic_spacegroup(self):
             ["La", "Mn", "O", "O"],
             [
                 [0.05, 0.25, 0.99],
-                [0.00, 0.00, 0.50],
+                [0, 0, 0.50],
                 [0.48, 0.25, 0.08],
                 [0.31, 0.04, 0.72],
             ],

pymatgen/ext/optimade.py

@@ -310,7 +310,7 @@ def get_snls_with_filter(
         response_fields = self._handle_response_fields(additional_response_fields)
 
         for identifier, resource in self.resources.items():
-            url = join(resource, f"v1/structures?filter={optimade_filter}&response_fields={response_fields}")
+            url = join(resource, f"v1/structures?filter={optimade_filter}&{response_fields=!s}")
 
             try:
                 json = self._get_json(url)

test_files/.pytest-split-durations

@@ -944,7 +944,7 @@
     "pymatgen/core/tests/test_sites.py::PeriodicSiteTest::test_repr": 0.0002163760073017329,
     "pymatgen/core/tests/test_sites.py::PeriodicSiteTest::test_setters": 0.0005762910150224343,
     "pymatgen/core/tests/test_sites.py::PeriodicSiteTest::test_to_unit_cell": 0.0004528739955276251,
-    "pymatgen/core/tests/test_sites.py::SiteTest::test_cmp": 0.00041866599349305034,
+    "pymatgen/core/tests/test_sites.py::SiteTest::test_gt_lt": 0.00041866599349305034,
     "pymatgen/core/tests/test_sites.py::SiteTest::test_distance": 0.0005102920113131404,
     "pymatgen/core/tests/test_sites.py::SiteTest::test_hash": 0.00025291601195931435,
     "pymatgen/core/tests/test_sites.py::SiteTest::test_pickle": 0.00037470700044650584,