Skip to content

Commit

Permalink
Avoid using eval, replace manual offset in enumerate and rename s…
Browse files Browse the repository at this point in the history
…ingle letter variables (#3739)

* `sourcery` tweaks

* replace usage of `eval`

* replace `eval`

* replace usage of `eval`

* document default angle_tolerance=5 degrees in SpacegroupAnalyzer

* rename single-letter variables in many files

* rename variables for readability

* use `start` arg in enumerate to replace manual offset

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
  • Loading branch information
DanielYang59 and janosh committed Apr 7, 2024
1 parent a2ab944 commit 2a3608f
Show file tree
Hide file tree
Showing 84 changed files with 416 additions and 417 deletions.
4 changes: 2 additions & 2 deletions pymatgen/analysis/bond_dissociation.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ def __init__(
" are less reliable! This is a bad idea!"
)
self.bond_pairs = []
for ii, bond in enumerate(self.ring_bonds):
for jj in range(ii + 1, len(self.ring_bonds)):
for ii, bond in enumerate(self.ring_bonds, start=1):
for jj in range(ii, len(self.ring_bonds)):
bond_pair = [bond, self.ring_bonds[jj]]
self.bond_pairs += [bond_pair]
for bond_pair in self.bond_pairs:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,11 @@ def elastic_centered_graph(self, start_node=None):
check_centered_connected_subgraph = nx.MultiGraph()
check_centered_connected_subgraph.add_nodes_from(centered_connected_subgraph.nodes())
check_centered_connected_subgraph.add_edges_from(
[e for e in centered_connected_subgraph.edges(data=True) if np.allclose(e[2]["delta"], np.zeros(3))]
[
edge
for edge in centered_connected_subgraph.edges(data=True)
if np.allclose(edge[2]["delta"], np.zeros(3))
]
)
if not is_connected(check_centered_connected_subgraph):
raise RuntimeError("Could not find a centered graph.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ def faces(self, sites, permutation=None):
list of its vertices coordinates.
"""
coords = [site.coords for site in sites] if permutation is None else [sites[ii].coords for ii in permutation]
return [[coords[ii] for ii in f] for f in self._faces]
return [[coords[ii] for ii in face] for face in self._faces]

def edges(self, sites, permutation=None, input="sites"):
"""
Expand All @@ -814,7 +814,7 @@ def edges(self, sites, permutation=None, input="sites"):
# coords = [sites[ii].coords for ii in permutation]
if permutation is not None:
coords = [coords[ii] for ii in permutation]
return [[coords[ii] for ii in e] for e in self._edges]
return [[coords[ii] for ii in edge] for edge in self._edges]

def solid_angles(self, permutation=None):
"""
Expand Down Expand Up @@ -854,11 +854,11 @@ def get_pmeshes(self, sites, permutation=None):
elif len(face) == 4:
out += "5\n"
else:
for ii, f in enumerate(face):
for ii, f in enumerate(face, start=1):
out += "4\n"
out += f"{len(_vertices) + iface}\n"
out += f"{f}\n"
out += f"{face[np.mod(ii + 1, len(face))]}\n"
out += f"{face[np.mod(ii, len(face))]}\n"
out += f"{len(_vertices) + iface}\n"
if len(face) in [3, 4]:
for face_vertex in face:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ def init_neighbors_sets(self, isite, additional_conditions=None, valences=None):
}
site_voronoi_indices = [
inb
for inb, voro_nb_dict in enumerate(site_voronoi)
for inb, _voro_nb_dict in enumerate(site_voronoi)
if (
distance_conditions[idp][inb]
and angle_conditions[iap][inb]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ def setup_voronoi_list(self, indices, voronoi_cutoff):
logging.debug("Please consider increasing voronoi_distance_cutoff")
t1 = time.process_time()
logging.debug("Setting up Voronoi list :")
for jj, isite in enumerate(indices):
logging.debug(f" - Voronoi analysis for site #{isite} ({jj + 1}/{len(indices)})")
for jj, isite in enumerate(indices, start=1):
logging.debug(f" - Voronoi analysis for site #{isite} ({jj}/{len(indices)})")
site = self.structure[isite]
neighbors1 = [(site, 0.0, isite)]
neighbors1.extend(struct_neighbors[isite])
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/chemenv/utils/chemenv_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def setup_package_options(self):
self.package_options = self.DEFAULT_PACKAGE_OPTIONS
print("Choose between the following strategies : ")
strategies = list(strategies_class_lookup)
for idx, strategy in enumerate(strategies, 1):
for idx, strategy in enumerate(strategies, start=1):
print(f" <{idx}> : {strategy}")
test = input(" ... ")
self.package_options["default_strategy"] = {
Expand Down
6 changes: 3 additions & 3 deletions pymatgen/analysis/chemenv/utils/graph_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def from_edges(cls, edges, edges_are_ordered: bool = True) -> Self:
order in the list.
"""
if edges_are_ordered:
nodes = [e[0] for e in edges]
nodes = [edge[0] for edge in edges]
if not all(e1e2[0][1] == e1e2[1][0] for e1e2 in zip(edges, edges[1:])) or edges[-1][1] != edges[0][0]:
raise ValueError("Could not construct a cycle from edges.")
else:
Expand Down Expand Up @@ -490,8 +490,8 @@ def get_all_elementary_cycles(graph):
edge_idx += 1
cycles_matrix = np.zeros(shape=(len(cycle_basis), edge_idx), dtype=bool)
for icycle, cycle in enumerate(cycle_basis):
for in1, n1 in enumerate(cycle):
n2 = cycle[(in1 + 1) % len(cycle)]
for in1, n1 in enumerate(cycle, start=1):
n2 = cycle[(in1) % len(cycle)]
iedge = all_edges_dict[(n1, n2)]
cycles_matrix[icycle, iedge] = True

Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/diffraction/tem.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ def get_plot_2d(self, structure: Structure) -> go.Figure:
),
]
layout = dict(
title="2D Diffraction Pattern<br>Beam Direction: " + "".join(str(e) for e in self.beam_direction),
title="2D Diffraction Pattern<br>Beam Direction: " + "".join(map(str, self.beam_direction)),
font={"size": 14, "color": "#7f7f7f"},
hovermode="closest",
xaxis={
Expand Down
8 changes: 4 additions & 4 deletions pymatgen/analysis/elasticity/elastic.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def snyder_ac(self, structure: Structure) -> float:
n_sites = len(structure)
n_atoms = structure.composition.num_atoms
site_density = 1e30 * n_sites / structure.volume
tot_mass = sum(e.atomic_mass for e in structure.species)
tot_mass = sum(spec.atomic_mass for spec in structure.species)
avg_mass = 1.6605e-27 * tot_mass / n_atoms
return (
0.38483
Expand Down Expand Up @@ -330,7 +330,7 @@ def clarke_thermalcond(self, structure: Structure) -> float:
float: Clarke's thermal conductivity (in SI units)
"""
n_sites = len(structure)
tot_mass = sum(e.atomic_mass for e in structure.species)
tot_mass = sum(spec.atomic_mass for spec in structure.species)
n_atoms = structure.composition.num_atoms
weight = float(structure.composition.weight)
avg_mass = 1.6605e-27 * tot_mass / n_atoms
Expand Down Expand Up @@ -761,8 +761,8 @@ def get_strain_from_stress(self, stress):
"""
compl_exp = self.get_compliance_expansion()
strain = 0
for n, compl in enumerate(compl_exp):
strain += compl.einsum_sequence([stress] * (n + 1)) / factorial(n + 1)
for n, compl in enumerate(compl_exp, start=1):
strain += compl.einsum_sequence([stress] * (n)) / factorial(n)
return strain

def get_effective_ecs(self, strain, order=2):
Expand Down
8 changes: 4 additions & 4 deletions pymatgen/analysis/graphs.py
Original file line number Diff line number Diff line change
Expand Up @@ -2156,8 +2156,8 @@ def build_unique_fragments(self):
unique_frags = []
for frag in fragments:
found = False
for f in unique_frags:
if _isomorphic(frag, f):
for fragment in unique_frags:
if _isomorphic(frag, fragment):
found = True
break
if not found:
Expand Down Expand Up @@ -2438,8 +2438,8 @@ def find_rings(self, including=None) -> list[list[tuple[int, int]]]:

for cycle in cycles_nodes:
edges = []
for idx, itm in enumerate(cycle):
edges.append((cycle[idx - 1], itm))
for idx, itm in enumerate(cycle, start=-1):
edges.append((cycle[idx], itm))
cycles_edges.append(edges)

return cycles_edges
Expand Down
4 changes: 2 additions & 2 deletions pymatgen/analysis/local_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -3337,8 +3337,8 @@ def get_order_parameters(
if self._geomops2:
# Compute all (unique) angles and sort the resulting list.
aij = []
for ir, r in enumerate(rij_norm):
for j in range(ir + 1, len(rij_norm)):
for ir, r in enumerate(rij_norm, start=1):
for j in range(ir, len(rij_norm)):
aij.append(acos(max(-1.0, min(np.inner(r, rij_norm[j]), 1.0))))
aijs = sorted(aij)

Expand Down
20 changes: 10 additions & 10 deletions pymatgen/analysis/magnetism/heisenberg.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@ def _get_nn_dict(self):
# Keep only up to NNNN and call dists equal if they are within tol
all_dists = sorted(set(all_dists))
rm_list = []
for idx, d in enumerate(all_dists[:-1]):
if abs(d - all_dists[idx + 1]) < tol:
rm_list.append(idx + 1)
for idx, d in enumerate(all_dists[:-1], start=1):
if abs(d - all_dists[idx]) < tol:
rm_list.append(idx)

all_dists = [d for idx, d in enumerate(all_dists) if idx not in rm_list]

Expand Down Expand Up @@ -567,11 +567,10 @@ def get_interaction_graph(self, filename=None):

# Save to a json file if desired
if filename:
if filename.endswith(".json"):
dumpfn(igraph, filename)
else:
if not filename.endswith(".json"):
filename += ".json"
dumpfn(igraph, filename)

dumpfn(igraph, filename)

return igraph

Expand Down Expand Up @@ -729,8 +728,9 @@ def _do_cleanup(structures, energies):
# Check for duplicate / degenerate states (sometimes different initial
# configs relax to the same state)
remove_list = []
e_tol = 6 # 10^-6 eV/atom tol on energies

for idx, energy in enumerate(energies):
e_tol = 6 # 10^-6 eV/atom tol on energies
energy = round(energy, e_tol)
if idx not in remove_list:
for i_check, e_check in enumerate(energies):
Expand All @@ -746,7 +746,7 @@ def _do_cleanup(structures, energies):
# remove_list.append(idx)

# Remove duplicates
if len(remove_list) > 0:
if remove_list:
ordered_structures = [struct for idx, struct in enumerate(ordered_structures) if idx not in remove_list]
energies = [energy for idx, energy in enumerate(energies) if idx not in remove_list]

Expand Down Expand Up @@ -923,7 +923,7 @@ def from_dict(cls, dct: dict) -> Self:

# Reconstitute the exchange matrix DataFrame
try:
ex_mat = eval(dct["ex_mat"])
ex_mat = literal_eval(dct["ex_mat"])
ex_mat = pd.DataFrame.from_dict(ex_mat)
except SyntaxError: # if ex_mat is empty
ex_mat = pd.DataFrame(columns=["E", "E0"])
Expand Down

0 comments on commit 2a3608f

Please sign in to comment.