Skip to content

Commit

Permalink
Tweak variable names (#3019)
Browse files Browse the repository at this point in the history
* use modern f"{float:+}" for explicit + signs

* fix tests

* fix use of formula_double_format

* fix reduced_form = "".join(reduced_form + polyanion)  # type: ignore
E       TypeError: sequence item 3: expected str instance, int found

* fix            for ads in ads_strs:
>               label += f"{ads:+}"
E               ValueError: Sign not allowed in string format specifier
  • Loading branch information
janosh authored May 29, 2023
1 parent 45060ec commit 9e400f2
Show file tree
Hide file tree
Showing 23 changed files with 163 additions and 197 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def from_dict(cls, dd):
Returns: a SeparationPlane algorithm.
"""
eop = (
[np.array(eoperm) for eoperm in dd["explicit_optimized_permutations"]]
[np.array(eo_perm) for eo_perm in dd["explicit_optimized_permutations"]]
if ("explicit_optimized_permutations" in dd and dd["explicit_optimized_permutations"] is not None)
else None
)
Expand All @@ -356,9 +356,9 @@ def from_dict(cls, dd):

def __str__(self):
out = "Separation plane algorithm with the following reference separation :\n"
out += f"[{'-'.join(str(pp) for pp in [self.point_groups[0]])}] | "
out += f"[{'-'.join(str(pp) for pp in [self.plane_points])}] | "
out += f"[{'-'.join(str(pp) for pp in [self.point_groups[1]])}]"
out += f"[{'-'.join(map(str, [self.point_groups[0]]))}] | "
out += f"[{'-'.join(map(str, [self.plane_points]))}] | "
out += f"[{'-'.join(map(str, [self.point_groups[1]]))}]"
return out


Expand Down Expand Up @@ -451,11 +451,7 @@ def double_cap_hints(self, hints_info):
new_site_voronoi_indices2.remove(second_cap_voronoi_index)
new_site_voronoi_indices3.remove(first_cap_voronoi_index)
new_site_voronoi_indices3.remove(second_cap_voronoi_index)
return [
new_site_voronoi_indices1,
new_site_voronoi_indices2,
new_site_voronoi_indices3,
]
return new_site_voronoi_indices1, new_site_voronoi_indices2, new_site_voronoi_indices3

def triple_cap_hints(self, hints_info):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1031,16 +1031,16 @@ def setup_test_perfect_environment(
RR = [[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]
else:
RR = random_rotation
newcoords = []
new_coords = []
for cc in coords:
newcc = np.dot(RR, cc).T
newcoords.append(newcc.ravel())
coords = newcoords
newcoords = []
new_coords.append(newcc.ravel())
coords = new_coords
new_coords = []
for cc in neighb_coords:
newcc = np.dot(RR, cc.T)
newcoords.append(newcc.ravel())
neighb_coords = newcoords
new_coords.append(newcc.ravel())
neighb_coords = new_coords

# Translating the test environment
if random_translation == "RANDOM":
Expand Down
18 changes: 7 additions & 11 deletions pymatgen/analysis/graphs.py
Original file line number Diff line number Diff line change
Expand Up @@ -2688,24 +2688,20 @@ def as_dict(self):
with using `to_dict_of_dicts` from NetworkX
to store graph information.
"""
d = {
"@module": type(self).__module__,
"@class": type(self).__name__,
"molecule": self.molecule.as_dict(),
"graphs": json_graph.adjacency_data(self.graph),
}

return d
dct = {"@module": type(self).__module__, "@class": type(self).__name__}
dct["molecule"] = self.molecule.as_dict()
dct["graphs"] = json_graph.adjacency_data(self.graph)
return dct

@classmethod
def from_dict(cls, d):
def from_dict(cls, dct):
"""
As in :class:`pymatgen.core.Molecule` except
restoring graphs using `from_dict_of_dicts`
from NetworkX to restore graph information.
"""
m = Molecule.from_dict(d["molecule"])
return cls(m, d["graphs"])
m = Molecule.from_dict(dct["molecule"])
return cls(m, dct["graphs"])

@classmethod
def _edges_to_string(cls, g):
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/phase_diagram.py
Original file line number Diff line number Diff line change
Expand Up @@ -2989,7 +2989,7 @@ def get_marker_props(coords, entries, stable=True):
e_above_hull = round(self._pd.get_e_above_hull(entry), 3)
if e_above_hull > self.show_unstable:
continue
label += f" (+{e_above_hull} eV/atom)"
label += f" ({e_above_hull:+} eV/atom)"
energies.append(e_above_hull)
else:
uncertainty = 0
Expand Down
42 changes: 21 additions & 21 deletions pymatgen/analysis/surface_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,18 @@ def as_dict(self):

def gibbs_binding_energy(self, eads=False):
"""
Returns the adsorption energy or Gibb's binding energy
Returns the adsorption energy or Gibbs binding energy
of an adsorbate on a surface
Args:
eads (bool): Whether to calculate the adsorption energy
(True) or the binding energy (False) which is just
adsorption energy normalized by number of adsorbates.
"""
n = self.get_unit_primitive_area
Nads = self.Nads_in_slab
n_ads = self.Nads_in_slab

BE = (self.energy - n * self.clean_entry.energy) / Nads - sum(ads.energy_per_atom for ads in self.adsorbates)
return BE * Nads if eads else BE
BE = (self.energy - n * self.clean_entry.energy) / n_ads - sum(ads.energy_per_atom for ads in self.adsorbates)
return BE * n_ads if eads else BE

def surface_energy(self, ucell_entry, ref_entries=None):
"""
Expand Down Expand Up @@ -211,7 +211,7 @@ def surface_energy(self, ucell_entry, ref_entries=None):
ref_entries_dict.update(self.ads_entries_dict)

# Calculate Gibbs free energy of the bulk per unit formula
gbulk = ucell_entry.energy / ucell_comp.get_integer_formula_and_factor()[1]
gibbs_bulk = ucell_entry.energy / ucell_comp.get_integer_formula_and_factor()[1]

# First we get the contribution to the bulk energy
# from each element with an existing ref_entry.
Expand All @@ -228,8 +228,8 @@ def surface_energy(self, ucell_entry, ref_entries=None):
for ref_el in ucell_comp.as_dict():
if str(ref_el) not in ref_entries_dict:
break
refEperA = (gbulk - gbulk_eqn) / ucell_reduced_comp.as_dict()[ref_el]
bulk_energy += self.composition.as_dict()[ref_el] * refEperA
ref_e_per_a = (gibbs_bulk - gbulk_eqn) / ucell_reduced_comp.as_dict()[ref_el]
bulk_energy += self.composition.as_dict()[ref_el] * ref_e_per_a
se = gamma.subs(
{
Symbol("E_surf"): self.energy,
Expand Down Expand Up @@ -258,9 +258,9 @@ def get_monolayer(self):
adsorbate.
"""
unit_a = self.get_unit_primitive_area
Nsurfs = self.Nsurfs_ads_in_slab
Nads = self.Nads_in_slab
return Nads / (unit_a * Nsurfs)
n_surfs = self.Nsurfs_ads_in_slab
n_ads = self.Nads_in_slab
return n_ads / (unit_a * n_surfs)

@property
def Nads_in_slab(self):
Expand All @@ -278,31 +278,31 @@ def Nsurfs_ads_in_slab(self):
weights = [s.species.weight for s in struct]
center_of_mass = np.average(struct.frac_coords, weights=weights, axis=0)

Nsurfs = 0
n_surfs = 0
# Are there adsorbates on top surface?
if any(
site.species_string in self.ads_entries_dict for site in struct if site.frac_coords[2] > center_of_mass[2]
):
Nsurfs += 1
n_surfs += 1
# Are there adsorbates on bottom surface?
if any(
site.species_string in self.ads_entries_dict for site in struct if site.frac_coords[2] < center_of_mass[2]
):
Nsurfs += 1
n_surfs += 1

return Nsurfs
return n_surfs

@classmethod
def from_dict(cls, d):
def from_dict(cls, dct):
"""
Returns a SlabEntry by reading in an dictionary
"""
structure = SlabEntry.from_dict(d["structure"])
energy = SlabEntry.from_dict(d["energy"])
miller_index = d["miller_index"]
label = d["label"]
adsorbates = d["adsorbates"]
clean_entry = d["clean_entry"]
structure = SlabEntry.from_dict(dct["structure"])
energy = SlabEntry.from_dict(dct["energy"])
miller_index = dct["miller_index"]
label = dct["label"]
adsorbates = dct["adsorbates"]
clean_entry = dct["clean_entry"]

return cls(
structure,
Expand Down
12 changes: 6 additions & 6 deletions pymatgen/command_line/tests/test_mcsqs_caller.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
@unittest.skipIf(not (which("mcsqs") and which("str2cif")), "mcsqs executable not present")
class McsqsCallerTest(PymatgenTest):
def setUp(self):
self.pztstructs = loadfn(os.path.join(test_dir, "pztstructs.json"))
self.pztstructs2 = loadfn(os.path.join(test_dir, "pztstructs2.json"))
self.pzt_structs = loadfn(os.path.join(test_dir, "pztstructs.json"))
self.pzt_structs2 = loadfn(os.path.join(test_dir, "pztstructs2.json"))
self.struct = self.get_structure("Pb2TiZrO6")
self.perfect_match_zzn_rs = loadfn(os.path.join(test_dir, "perfect_match_zzn_rs.json"))

Expand All @@ -32,7 +32,7 @@ def test_mcsqs_caller_supercell(self):
struct.replace_species({"Ti": {"Ti": 0.5, "Zr": 0.5}, "Zr": {"Ti": 0.5, "Zr": 0.5}})
sqs = run_mcsqs(struct, {2: 6, 3: 4}, scaling=[2, 1, 1], search_time=0.01, instances=1)

matches = [sqs.bestsqs.matches(s) for s in self.pztstructs]
matches = [sqs.bestsqs.matches(s) for s in self.pzt_structs]
assert True in matches

assert isinstance(sqs.bestsqs, Structure)
Expand All @@ -51,15 +51,15 @@ def test_mcsqs_caller_total_atoms(self):
struct.replace_species({"Ti": {"Ti": 0.5, "Zr": 0.5}, "Zr": {"Ti": 0.5, "Zr": 0.5}})
sqs = run_mcsqs(struct, {2: 6, 3: 4}, scaling=2, search_time=0.01, instances=1)

matches = [sqs.bestsqs.matches(s) for s in self.pztstructs2]
matches = [sqs.bestsqs.matches(s) for s in self.pzt_structs2]
assert True in matches

def test_mcsqs_caller_total_atoms_auto_instances(self):
struct = self.struct.copy()
struct.replace_species({"Ti": {"Ti": 0.5, "Zr": 0.5}, "Zr": {"Ti": 0.5, "Zr": 0.5}})
sqs = run_mcsqs(struct, {2: 6, 3: 4}, scaling=2, search_time=0.01, instances=None)

matches = [sqs.bestsqs.matches(s) for s in self.pztstructs2]
matches = [sqs.bestsqs.matches(s) for s in self.pzt_structs2]
assert True in matches

def test_mcsqs_caller_parallel(self):
Expand All @@ -69,7 +69,7 @@ def test_mcsqs_caller_parallel(self):
struct.replace_species({"Ti": {"Ti": 0.5, "Zr": 0.5}, "Zr": {"Ti": 0.5, "Zr": 0.5}})
sqs = run_mcsqs(struct, {2: 6, 3: 4}, scaling=2, search_time=0.01, instances=4)

matches = [sqs.bestsqs.matches(s) for s in self.pztstructs2]
matches = [sqs.bestsqs.matches(s) for s in self.pzt_structs2]
assert True in matches

def test_mcsqs_perfect_match_error(self):
Expand Down
15 changes: 7 additions & 8 deletions pymatgen/core/composition.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def formula(self) -> str:
"""
sym_amt = self.get_el_amt_dict()
syms = sorted(sym_amt, key=lambda sym: get_el_sp(sym).X)
formula = [s + formula_double_format(sym_amt[s], False) for s in syms]
formula = [f"{s}{formula_double_format(sym_amt[s], False)}" for s in syms]
return " ".join(formula)

@property
Expand All @@ -317,7 +317,7 @@ def iupac_formula(self) -> str:
"""
sym_amt = self.get_el_amt_dict()
syms = sorted(sym_amt, key=lambda s: get_el_sp(s).iupac_ordering)
formula = [s + formula_double_format(sym_amt[s], False) for s in syms]
formula = [f"{s}{formula_double_format(sym_amt[s], False)}" for s in syms]
return " ".join(formula)

@property
Expand Down Expand Up @@ -1231,13 +1231,12 @@ def reduce_formula(sym_amt, iupac_ordering: bool = False) -> tuple[str, float]:
syms = sorted(syms, key=lambda x: [get_el_sp(x).iupac_ordering, x])

reduced_form = []
for s in syms:
normamt = sym_amt[s] * 1.0 / factor
reduced_form.append(s)
reduced_form.append(formula_double_format(normamt))
for sym in syms:
norm_amt = sym_amt[sym] * 1.0 / factor
reduced_form.append(sym)
reduced_form.append(str(formula_double_format(norm_amt)))

reduced_form = "".join(reduced_form + polyanion) # type: ignore
return reduced_form, factor # type: ignore
return "".join([*reduced_form, *polyanion]), factor


class ChemicalPotential(dict, MSONable):
Expand Down
6 changes: 2 additions & 4 deletions pymatgen/core/ion.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,6 @@ def to_pretty_string(self) -> str:
:return: Pretty string with proper superscripts.
"""
str_ = super().reduced_formula
if self.charge > 0:
str_ += "^+" + formula_double_format(self.charge, False)
elif self._charge < 0:
str_ += "^" + formula_double_format(self.charge, False)
if val := formula_double_format(self.charge, False):
str_ += f"^{val:+}"
return str_
8 changes: 6 additions & 2 deletions pymatgen/core/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,14 @@ def __init__(self, affine_transformation_matrix: ArrayLike, tol: float = 0.01) -
affine_transformation_matrix (4x4 array): Representing an
affine transformation.
tol (float): Tolerance for determining if matrices are equal.
Raises:
ValueError: if matrix is not 4x4.
"""
affine_transformation_matrix = np.array(affine_transformation_matrix)
if affine_transformation_matrix.shape != (4, 4):
raise ValueError("Affine Matrix must be a 4x4 numpy array!")
shape = affine_transformation_matrix.shape
if shape != (4, 4):
raise ValueError(f"Affine Matrix must be a 4x4 numpy array, got {shape=}")
self.affine_matrix = affine_transformation_matrix
self.tol = tol

Expand Down
23 changes: 7 additions & 16 deletions pymatgen/core/periodic_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -1222,12 +1222,9 @@ def __repr__(self):
def __str__(self):
output = self.symbol
if self.oxi_state is not None:
if self.oxi_state >= 0:
output += formula_double_format(self.oxi_state) + "+"
else:
output += formula_double_format(-self.oxi_state) + "-"
for p, v in self._properties.items():
output += f",{p}={v}"
output += f"{formula_double_format(abs(self.oxi_state))}{'+' if self.oxi_state >= 0 else '-'}"
for prop, val in self._properties.items():
output += f",{prop}={val}"
return output

def to_pretty_string(self) -> str:
Expand All @@ -1236,10 +1233,7 @@ def to_pretty_string(self) -> str:
"""
output = self.symbol
if self.oxi_state is not None:
if self.oxi_state >= 0:
output += formula_double_format(self.oxi_state) + "+"
else:
output += formula_double_format(-self.oxi_state) + "-"
output += f"{formula_double_format(abs(self.oxi_state))}{'+' if self.oxi_state >= 0 else '-'}"
return output

def get_nmr_quadrupole_moment(self, isotope: str | None = None) -> float:
Expand Down Expand Up @@ -1547,12 +1541,9 @@ def __repr__(self):
def __str__(self):
output = self.symbol
if self.oxi_state is not None:
if self.oxi_state >= 0:
output += formula_double_format(self.oxi_state) + "+"
else:
output += formula_double_format(-self.oxi_state) + "-"
for p, v in self._properties.items():
output += f",{p}={v}"
output += f"{formula_double_format(abs(self.oxi_state))}{'+' if self.oxi_state >= 0 else '-'}"
for prop, val in self._properties.items():
output += f",{prop}={val}"
return output


Expand Down
Loading

0 comments on commit 9e400f2

Please sign in to comment.