add_oxidation_state_by_guess clarify which guess is used if multiple

fix doc strings with empty return value section

Author: janosh

pymatgen/analysis/chemenv/connectivity/connected_components.py

@@ -371,12 +371,10 @@ def coordination_sequence(self, source_node, path_size=5, coordination="number",
     def __len__(self):
         return len(self.graph)
 
-    def compute_periodicity(self, algorithm="all_simple_paths"):
+    def compute_periodicity(self, algorithm="all_simple_paths") -> None:
         """
         Args:
             algorithm ():
-
-        Returns:
         """
         if algorithm == "all_simple_paths":
             self.compute_periodicity_all_simple_paths_algorithm()
@@ -519,6 +517,7 @@ def make_supergraph(self, multiplicity):
             multiplicity ():
 
         Returns:
+            nx.MultiGraph: Super graph of the connected component.
         """
         return make_supergraph(self._connected_subgraph, multiplicity, self._periodicity_vectors)
 

pymatgen/analysis/chemenv/connectivity/structure_connectivity.py

@@ -50,14 +50,14 @@ def __init__(
 
         Args:
             light_structure_environment: a LightStructureEnvironments object
-                                         containing the relevant local environments
-                                         for the sites in the structure.
+                containing the relevant local environments
+                for the sites in the structure.
             connectivity_graph: the networkx MultiGraph if it has already been computed,
-                                e.g. stored in a file or dict and StructureConnectivity
-                                is reconstructed from that file or dict.
+                e.g. stored in a file or dict and StructureConnectivity
+                is reconstructed from that file or dict.
             environment_subgraphs: the different subgraphs of environments that have
-                                   been computed if any (as for connectivity_graph, only
-                                   if it is reconstructed from a file or dict).
+                been computed if any (as for connectivity_graph, only
+                if it is reconstructed from a file or dict).
         """
         self.light_structure_environments = light_structure_environment
         if connectivity_graph is None:
@@ -76,6 +76,7 @@ def environment_subgraph(self, environments_symbols=None, only_atoms=None):
             only_atoms ():
 
         Returns:
+            nx.MultiGraph: The subgraph of the structure connectivity graph
         """
         if environments_symbols is not None:
             self.setup_environment_subgraph(environments_symbols=environments_symbols, only_atoms=only_atoms)
@@ -304,11 +305,12 @@ def from_dict(cls, d):
             d ():
 
         Returns:
+            StructureConnectivity
         """
         # Reconstructs the graph with integer as nodes (json's as_dict replaces integer keys with str keys)
         cgraph = nx.from_dict_of_dicts(d["connectivity_graph"], create_using=nx.MultiGraph, multigraph_input=True)
         cgraph = nx.relabel_nodes(cgraph, int)  # Just relabel the nodes using integer casting (maps str->int)
-        # Relabel multiedges (removes multiedges with str keys and adds them back with int keys)
+        # Relabel multi-edges (removes multi-edges with str keys and adds them back with int keys)
         edges = set(cgraph.edges())
         for n1, n2 in edges:
             new_edges = {int(iedge): edata for iedge, edata in cgraph[n1][n2].items()}

pymatgen/analysis/molecule_structure_comparator.py

@@ -192,6 +192,7 @@ def get_13_bonds(priority_bonds):
             priority_bonds ():
 
         Returns:
+            tuple: 13 bonds
         """
         all_bond_pairs = list(itertools.combinations(priority_bonds, r=2))
         all_2_bond_atoms = [set(b1 + b2) for b1, b2 in all_bond_pairs]
@@ -203,8 +204,7 @@ def get_13_bonds(priority_bonds):
         return tuple(sorted(bonds_13))
 
     def _get_bonds(self, mol):
-        """
-        Find all the bond in a molcule.
+        """Find all bonds in a molecule.
 
         Args:
             mol: the molecule. pymatgen Molecule object

pymatgen/analysis/pourbaix_diagram.py

@@ -809,6 +809,7 @@ def find_stable_entry(self, pH, V):
             V (float): V to find stable entry.
 
         Returns:
+            PourbaixEntry: stable entry at pH, V
         """
         energies_at_conditions = [e.normalized_energy_at_conditions(pH, V) for e in self.stable_entries]
         return self.stable_entries[np.argmin(energies_at_conditions)]

pymatgen/command_line/bader_caller.py

@@ -303,8 +303,8 @@ def get_oxidation_state_decorated_structure(self, nelects: list[int] | None = No
         Returns:
             Structure: with bader-analysis-based oxidation states.
         """
-        charges = [self.get_partial_charge(i, None if not nelects else nelects[i]) for i in range(len(self.structure))]
         struct = self.structure.copy()
+        charges = [self.get_partial_charge(idx, None if not nelects else nelects[idx]) for idx in range(len(struct))]
         struct.add_oxidation_state_by_site(charges)
         return struct
 

pymatgen/core/structure.py

@@ -570,7 +570,8 @@ def remove_oxidation_states(self) -> None:
 
     def add_oxidation_state_by_guess(self, **kwargs) -> None:
         """Decorates the structure with oxidation state, guessing
-        using Composition.oxi_state_guesses().
+        using Composition.oxi_state_guesses(). If multiple guesses are found
+        we take the first one.
 
         Args:
             **kwargs: parameters to pass into oxi_state_guesses()
@@ -1193,7 +1194,7 @@ def from_magnetic_spacegroup(
         return cls(latt, all_sp, all_coords, site_properties=all_site_properties, labels=all_labels)
 
     def unset_charge(self):
-        """Reset the charge to None, i.e., computed dynamically based on oxidation states."""
+        """Reset the charge to None. E.g. to compute it dynamically based on oxidation states."""
         self._charge = None
 
     @property
@@ -1222,10 +1223,10 @@ def charge(self) -> float:
         formal_charge = super().charge
         if self._charge is None:
             return super().charge
-        if formal_charge != self._charge:
+        if abs(formal_charge - self._charge) > 1e-8:
             warnings.warn(
                 f"Structure charge ({self._charge}) is set to be not equal to the sum of oxidation states"
-                f" ({formal_charge}). Use `unset_charge` if this is not desired."
+                f" ({formal_charge}). Use Structure.unset_charge() to reset the charge to None."
             )
         return self._charge
 

pymatgen/io/lobster/lobsterenv.py

@@ -685,7 +685,7 @@ def _evaluate_ce(
         additional_condition=0,
         perc_strength_ICOHP=0.15,
         adapt_extremum_to_add_cond=False,
-    ):
+    ) -> None:
         """
         Args:
             lowerlimit: lower limit which determines the ICOHPs that are considered for the determination of the
@@ -698,8 +698,6 @@ def _evaluate_ce(
             that are still considered for the evaluation
             adapt_extremum_to_add_cond: will recalculate the limit based on the bonding type and not on the overall
             extremum.
-
-        Returns:
         """
         # get extremum
         if lowerlimit is None and upperlimit is None:
@@ -812,6 +810,7 @@ def _find_environments(self, additional_condition, lowerlimit, upperlimit, only_
             only_bonds_to (list): list of str, e.g. ["O"] that will ensure that only bonds to "O" will be considered
 
         Returns:
+            tuple: list of icohps, list of keys, list of lengths, list of neighisite, list of neighsite, list of coords
         """
         # run over structure
         list_neighsite = []
@@ -829,12 +828,8 @@ def _find_environments(self, additional_condition, lowerlimit, upperlimit, only_
                 only_bonds_to=only_bonds_to,
             )
 
-            (
-                keys_from_ICOHPs,
-                lengths_from_ICOHPs,
-                neighbors_from_ICOHPs,
-                selected_ICOHPs,
-            ) = self._find_relevant_atoms_additional_condition(idx, icohps, additional_condition)
+            additional_conds = self._find_relevant_atoms_additional_condition(idx, icohps, additional_condition)
+            keys_from_ICOHPs, lengths_from_ICOHPs, neighbors_from_ICOHPs, selected_ICOHPs = additional_conds
 
             if len(neighbors_from_ICOHPs) > 0:
                 centralsite = self.structure[idx]
@@ -927,6 +922,7 @@ def _find_relevant_atoms_additional_condition(self, isite, icohps, additional_co
             additional_condition (int): additional condition
 
         Returns:
+            tuple: keys, lengths and neighbors from selected ICOHPs and selected ICOHPs
         """
         neighbors_from_ICOHPs = []
         lengths_from_ICOHPs = []
@@ -1048,17 +1044,11 @@ def _find_relevant_atoms_additional_condition(self, isite, icohps, additional_co
                     icohps_from_ICOHPs.append(icohp.summed_icohp)
                     keys_from_ICOHPs.append(key)
 
-        return (
-            keys_from_ICOHPs,
-            lengths_from_ICOHPs,
-            neighbors_from_ICOHPs,
-            icohps_from_ICOHPs,
-        )
+        return keys_from_ICOHPs, lengths_from_ICOHPs, neighbors_from_ICOHPs, icohps_from_ICOHPs
 
     @staticmethod
     def _get_icohps(icohpcollection, isite, lowerlimit, upperlimit, only_bonds_to):
-        """
-        Return icohp dict for certain site.
+        """Return icohp dict for certain site.
 
         Args:
             icohpcollection: Icohpcollection object
@@ -1068,6 +1058,7 @@ def _get_icohps(icohpcollection, isite, lowerlimit, upperlimit, only_bonds_to):
             only_bonds_to (list): list of str, e.g. ["O"] that will ensure that only bonds to "O" will be considered
 
         Returns:
+            dict: of IcohpValues. The keys correspond to the values from the initial list_labels.
         """
         return icohpcollection.get_icohp_dict_of_site(
             site=isite,
@@ -1091,7 +1082,7 @@ def _get_atomnumber(atomstring) -> int:
         return int(LobsterNeighbors._split_string(atomstring)[1]) - 1
 
     @staticmethod
-    def _split_string(s):
+    def _split_string(s) -> tuple[str, str]:
         """
         Will split strings such as "Na1" in "Na" and "1" and return "1".
 
@@ -1140,7 +1131,7 @@ def _get_limit_from_extremum(
     ):
         """
         Return limits for the evaluation of the icohp values from an icohpcollection
-        Return -float('inf'), min(max_icohp*0.15,-0.1). Currently only works for ICOHPs.
+        Return -float("inf"), min(max_icohp*0.15,-0.1). Currently only works for ICOHPs.
 
         Args:
             icohpcollection: icohpcollection object
@@ -1390,10 +1381,10 @@ class ICOHPNeighborsInfo(NamedTuple):
         total_icohp (float): sum of icohp values of neighbors to the selected sites [given by the id in structure]
         list_icohps (list): list of summed icohp values for all identified interactions with neighbors
         n_bonds (int): number of identified bonds to the selected sites
-        labels (list(str)): labels (from ICOHPLIST) for all identified bonds
-        atoms (list(list(str)): list of list describing the species present in the identified interactions
-            (names from ICOHPLIST), e.g., ['Ag3', 'O5']
-        central_isites (list(int)): list of the central isite for each identified interaction.
+        labels (list[str]): labels (from ICOHPLIST) for all identified bonds
+        atoms (list[list[str]]): list of list describing the species present in the identified interactions
+            (names from ICOHPLIST), e.g., ["Ag3", "O5"]
+        central_isites (list[int]): list of the central isite for each identified interaction.
 
 
     """