Skip to content

Commit

Permalink
add_oxidation_state_by_guess clarify which guess is used if multiple
Browse files Browse the repository at this point in the history
fix doc strings with empty return value section
  • Loading branch information
janosh committed Nov 7, 2023
1 parent abd9893 commit b3ac45a
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)

Expand Down
16 changes: 9 additions & 7 deletions pymatgen/analysis/chemenv/connectivity/structure_connectivity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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()}
Expand Down
4 changes: 2 additions & 2 deletions pymatgen/analysis/molecule_structure_comparator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions pymatgen/analysis/pourbaix_diagram.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/command_line/bader_caller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 5 additions & 4 deletions pymatgen/core/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
37 changes: 14 additions & 23 deletions pymatgen/io/lobster/lobsterenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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 = []
Expand All @@ -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]
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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".
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
"""
Expand Down

0 comments on commit b3ac45a

Please sign in to comment.