Skip to content

Commit

Permalink
Deprecate overlooked from/as_..._string methods (#3295)
Browse files Browse the repository at this point in the history
* rename variables: toks->tokens

* add missing from/as_<whatever>_string deprecations
  • Loading branch information
janosh authored Sep 2, 2023
1 parent 49083cc commit 387dbf4
Show file tree
Hide file tree
Showing 21 changed files with 290 additions and 266 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
- name: ruff
run: |
ruff --version
ruff . --ignore D
ruff .
- name: black
run: |
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ ci:

repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.286
rev: v0.0.287
hooks:
- id: ruff
args: [--fix]
Expand Down
26 changes: 13 additions & 13 deletions dev_scripts/update_pt_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,16 @@ def parse_ionic_radii():
header = radii_data[0].split(",")
for idx in range(1, len(radii_data)):
line = radii_data[idx]
toks = line.strip().split(",")
tokens = line.strip().split(",")
suffix = ""
name = toks[1]
name = tokens[1]
if len(name.split(" ")) > 1:
suffix = "_" + name.split(" ")[1]
el = toks[2]
el = tokens[2]

ionic_radii = {}
for j in range(3, len(toks)):
m = re.match(r"^\s*([0-9\.]+)", toks[j])
for j in range(3, len(tokens)):
m = re.match(r"^\s*([0-9\.]+)", tokens[j])
if m:
ionic_radii[int(header[j])] = float(m.group(1))

Expand All @@ -109,22 +109,22 @@ def parse_radii():
radii_data = radii_data.split("\r")

for line in radii_data:
toks = line.strip().split(",")
el = toks[1]
tokens = line.strip().split(",")
el = tokens[1]
try:
atomic_radii = float(toks[3]) / 100
atomic_radii = float(tokens[3]) / 100
except Exception:
atomic_radii = toks[3]
atomic_radii = tokens[3]

try:
atomic_radii_calc = float(toks[4]) / 100
atomic_radii_calc = float(tokens[4]) / 100
except Exception:
atomic_radii_calc = toks[4]
atomic_radii_calc = tokens[4]

try:
vdw_radii = float(toks[5]) / 100
vdw_radii = float(tokens[5]) / 100
except Exception:
vdw_radii = toks[5]
vdw_radii = tokens[5]

if el in data:
data[el]["Atomic radius"] = atomic_radii
Expand Down
46 changes: 29 additions & 17 deletions pymatgen/alchemy/materials.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from typing import TYPE_CHECKING, Any
from warnings import warn

import numpy as np
from monty.json import MSONable, jsanitize

from pymatgen.core.structure import Structure
Expand Down Expand Up @@ -244,8 +245,14 @@ def structures(self) -> list[Structure]:
h_structs = [Structure.from_dict(s["input_structure"]) for s in self.history if "input_structure" in s]
return [*h_structs, self.final_structure]

@staticmethod
def from_cif_string(
@classmethod
@np.deprecate(message="Use from_cif_str instead")
def from_cif_string(cls, *args, **kwargs): # noqa: D102
return cls.from_cif_str(*args, **kwargs)

@classmethod
def from_cif_str(
cls,
cif_string: str,
transformations: list[AbstractTransformation] | None = None,
primitive: bool = True,
Expand Down Expand Up @@ -287,11 +294,16 @@ def from_cif_string(
"original_file": raw_string,
"cif_data": cif_dict[cif_keys[0]],
}
return TransformedStructure(struct, transformations, history=[source_info])
return cls(struct, transformations, history=[source_info])

@staticmethod
def from_poscar_string(
poscar_string: str, transformations: list[AbstractTransformation] | None = None
@classmethod
@np.deprecate(message="Use from_poscar_str instead")
def from_poscar_string(cls, *args, **kwargs): # noqa: D102
return cls.from_poscar_str(*args, **kwargs)

@classmethod
def from_poscar_str(
cls, poscar_string: str, transformations: list[AbstractTransformation] | None = None
) -> TransformedStructure:
"""Generates TransformedStructure from a poscar string.
Expand All @@ -300,29 +312,29 @@ def from_poscar_string(
transformations (list[Transformation]): Sequence of transformations
to be applied to the input structure.
"""
p = Poscar.from_str(poscar_string)
if not p.true_names:
poscar = Poscar.from_str(poscar_string)
if not poscar.true_names:
raise ValueError(
"Transformation can be created only from POSCAR strings with proper VASP5 element symbols."
)
raw_string = re.sub(r"'", '"', poscar_string)
struct = p.structure
struct = poscar.structure
source_info = {
"source": "POSCAR",
"datetime": str(datetime.datetime.now()),
"original_file": raw_string,
}
return TransformedStructure(struct, transformations, history=[source_info])
return cls(struct, transformations, history=[source_info])

def as_dict(self) -> dict[str, Any]:
"""Dict representation of the TransformedStructure."""
d = self.final_structure.as_dict()
d["@module"] = type(self).__module__
d["@class"] = type(self).__name__
d["history"] = jsanitize(self.history)
d["last_modified"] = str(datetime.datetime.utcnow())
d["other_parameters"] = jsanitize(self.other_parameters)
return d
dct = self.final_structure.as_dict()
dct["@module"] = type(self).__module__
dct["@class"] = type(self).__name__
dct["history"] = jsanitize(self.history)
dct["last_modified"] = str(datetime.datetime.utcnow())
dct["other_parameters"] = jsanitize(self.other_parameters)
return dct

@classmethod
def from_dict(cls, d) -> TransformedStructure:
Expand Down
34 changes: 17 additions & 17 deletions pymatgen/alchemy/transmuters.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ def append_transformation(self, transformation, extend_collection=False, clear_r
with Pool(self.ncores) as p:
# need to condense arguments into single tuple to use map
z = ((x, transformation, extend_collection, clear_redo) for x in self.transformed_structures)
new_tstructs = p.map(_apply_transformation, z, 1)
nrafo_ew_tstructs = p.map(_apply_transformation, z, 1)
self.transformed_structures = []
for ts in new_tstructs:
for ts in nrafo_ew_tstructs:
self.transformed_structures.extend(ts)
else:
new_structures = []
Expand Down Expand Up @@ -188,21 +188,21 @@ def __str__(self):
output.append(str(x.final_structure))
return "\n".join(output)

def append_transformed_structures(self, tstructs_or_transmuter):
def append_transformed_structures(self, trafo_structs_or_transmuter):
"""Method is overloaded to accept either a list of transformed structures
or transmuter, it which case it appends the second transmuter"s
structures.
Args:
tstructs_or_transmuter: A list of transformed structures or a
trafo_structs_or_transmuter: A list of transformed structures or a
transmuter.
"""
if isinstance(tstructs_or_transmuter, self.__class__):
self.transformed_structures.extend(tstructs_or_transmuter.transformed_structures)
if isinstance(trafo_structs_or_transmuter, self.__class__):
self.transformed_structures.extend(trafo_structs_or_transmuter.transformed_structures)
else:
for ts in tstructs_or_transmuter:
for ts in trafo_structs_or_transmuter:
assert isinstance(ts, TransformedStructure)
self.transformed_structures.extend(tstructs_or_transmuter)
self.transformed_structures.extend(trafo_structs_or_transmuter)

@staticmethod
def from_structures(structures, transformations=None, extend_collection=0):
Expand All @@ -221,8 +221,8 @@ def from_structures(structures, transformations=None, extend_collection=0):
Returns:
StandardTransmuter
"""
tstruct = [TransformedStructure(s, []) for s in structures]
return StandardTransmuter(tstruct, transformations, extend_collection)
trafo_struct = [TransformedStructure(s, []) for s in structures]
return StandardTransmuter(trafo_struct, transformations, extend_collection)


class CifTransmuter(StandardTransmuter):
Expand Down Expand Up @@ -255,8 +255,8 @@ def __init__(self, cif_string, transformations=None, primitive=True, extend_coll
if read_data:
structure_data[-1].append(line)
for data in structure_data:
tstruct = TransformedStructure.from_cif_string("\n".join(data), [], primitive)
transformed_structures.append(tstruct)
trafo_struct = TransformedStructure.from_cif_string("\n".join(data), [], primitive)
transformed_structures.append(trafo_struct)
super().__init__(transformed_structures, transformations, extend_collection)

@staticmethod
Expand Down Expand Up @@ -295,8 +295,8 @@ def __init__(self, poscar_string, transformations=None, extend_collection=False)
extend_collection: Whether to use more than one output structure
from one-to-many transformations.
"""
tstruct = TransformedStructure.from_poscar_string(poscar_string, [])
super().__init__([tstruct], transformations, extend_collection=extend_collection)
trafo_struct = TransformedStructure.from_poscar_string(poscar_string, [])
super().__init__([trafo_struct], transformations, extend_collection=extend_collection)

@staticmethod
def from_filenames(poscar_filenames, transformations=None, extend_collection=False):
Expand All @@ -310,11 +310,11 @@ def from_filenames(poscar_filenames, transformations=None, extend_collection=Fal
extend_collection:
Same meaning as in __init__.
"""
tstructs = []
trafo_structs = []
for filename in poscar_filenames:
with open(filename) as f:
tstructs.append(TransformedStructure.from_poscar_string(f.read(), []))
return StandardTransmuter(tstructs, transformations, extend_collection=extend_collection)
trafo_structs.append(TransformedStructure.from_poscar_string(f.read(), []))
return StandardTransmuter(trafo_structs, transformations, extend_collection=extend_collection)


def batch_write_vasp_input(
Expand Down
6 changes: 3 additions & 3 deletions pymatgen/cli/pmg_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ def analyze_localenv(args):
"""
bonds = {}
for bond in args.localenv:
toks = bond.split("=")
species = toks[0].split("-")
bonds[(species[0], species[1])] = float(toks[1])
tokens = bond.split("=")
species = tokens[0].split("-")
bonds[(species[0], species[1])] = float(tokens[1])
for filename in args.filenames:
print(f"Analyzing {filename}...")
data = []
Expand Down
28 changes: 18 additions & 10 deletions pymatgen/core/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,32 +418,40 @@ def as_dict(self) -> dict[str, Any]:
"tolerance": self.tol,
}

def as_xyz_string(self) -> str:
"""Returns a string of the form 'x, y, z', '-x, -y, z',
'-y+1/2, x+1/2, z+1/2', etc. Only works for integer rotation matrices.
@np.deprecate(message="Use as_xyz_str instead")
def as_xyz_string(cls, *args, **kwargs): # noqa: D102
return cls.as_xyz_str(*args, **kwargs)

def as_xyz_str(self) -> str:
"""Returns a string of the form 'x, y, z', '-x, -y, z', '-y+1/2, x+1/2, z+1/2', etc.
Only works for integer rotation matrices.
"""
# test for invalid rotation matrix
if not np.all(np.isclose(self.rotation_matrix, np.round(self.rotation_matrix))):
warnings.warn("Rotation matrix should be integer")

return transformation_to_string(self.rotation_matrix, translation_vec=self.translation_vector, delim=", ")

@staticmethod
def from_xyz_string(xyz_string: str) -> SymmOp:
@classmethod
@np.deprecate(message="Use from_xyz_str instead")
def from_xyz_string(cls, *args, **kwargs): # noqa: D102
return cls.from_xyz_str(*args, **kwargs)

@classmethod
def from_xyz_str(cls, xyz_str: str) -> SymmOp:
"""
Args:
xyz_string: string of the form 'x, y, z', '-x, -y, z',
'-2y+1/2, 3x+1/2, z-y+1/2', etc.
xyz_str: string of the form 'x, y, z', '-x, -y, z', '-2y+1/2, 3x+1/2, z-y+1/2', etc.
Returns:
SymmOp
"""
rot_matrix = np.zeros((3, 3))
trans = np.zeros(3)
toks = xyz_string.strip().replace(" ", "").lower().split(",")
tokens = xyz_str.strip().replace(" ", "").lower().split(",")
re_rot = re.compile(r"([+-]?)([\d\.]*)/?([\d\.]*)([x-z])")
re_trans = re.compile(r"([+-]?)([\d\.]+)/?([\d\.]*)(?![x-z])")
for i, tok in enumerate(toks):
for i, tok in enumerate(tokens):
# build the rotation matrix
for m in re_rot.finditer(tok):
factor = -1.0 if m.group(1) == "-" else 1.0
Expand All @@ -456,7 +464,7 @@ def from_xyz_string(xyz_string: str) -> SymmOp:
factor = -1 if m.group(1) == "-" else 1
num = float(m.group(2)) / float(m.group(3)) if m.group(3) != "" else float(m.group(2))
trans[i] = num * factor
return SymmOp.from_rotation_and_translation(rot_matrix, trans)
return cls.from_rotation_and_translation(rot_matrix, trans)

@classmethod
def from_dict(cls, d) -> SymmOp:
Expand Down
8 changes: 4 additions & 4 deletions pymatgen/core/periodic_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -1143,8 +1143,8 @@ def from_str(species_string: str) -> Species:
# parse properties (optional)
properties = {}
if match.group(4): # has Spin properties
toks = match.group(4).replace(",", "").split("=")
properties = {toks[0]: ast.literal_eval(toks[1])}
tokens = match.group(4).replace(",", "").split("=")
properties = {tokens[0]: ast.literal_eval(tokens[1])}

# but we need either an oxidation state or a property
if oxi is None and properties == {}:
Expand Down Expand Up @@ -1422,8 +1422,8 @@ def from_str(species_string: str) -> DummySpecies:
oxi = -oxi if m.group(3) == "-" else oxi
properties = {}
if m.group(4): # has Spin property
toks = m.group(4).split("=")
properties = {toks[0]: float(toks[1])}
tokens = m.group(4).split("=")
properties = {tokens[0]: float(tokens[1])}
return DummySpecies(sym, oxi, **properties)
raise ValueError("Invalid DummySpecies String")

Expand Down
8 changes: 4 additions & 4 deletions pymatgen/io/cssr.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ def from_str(string):
Cssr object.
"""
lines = string.split("\n")
toks = lines[0].split()
lengths = [float(tok) for tok in toks]
toks = lines[1].split()
angles = [float(tok) for tok in toks[0:3]]
tokens = lines[0].split()
lengths = [float(tok) for tok in tokens]
tokens = lines[1].split()
angles = [float(tok) for tok in tokens[0:3]]
latt = Lattice.from_parameters(*lengths, *angles)
sp = []
coords = []
Expand Down
Loading

0 comments on commit 387dbf4

Please sign in to comment.