Skip to content

Commit

Permalink
more descriptive variable names
Browse files Browse the repository at this point in the history
  • Loading branch information
janosh committed May 14, 2024
1 parent a02cdea commit 90518e0
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 66 deletions.
4 changes: 2 additions & 2 deletions src/atomate2/aims/jobs/convergence.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ def make(
if prev_dir is not None:
split_prev_dir = str(prev_dir).split(":")[-1]
convergence_file = Path(split_prev_dir) / CONVERGENCE_FILE_NAME
with open(convergence_file, "w") as f:
json.dump(convergence_data, f)
with open(convergence_file, "w") as file:
json.dump(convergence_data, file)

if idx < len(self.convergence_steps) and not converged:
# finding next jobs
Expand Down
4 changes: 2 additions & 2 deletions src/atomate2/aims/schemas/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,8 @@ def from_aims_calc_doc(cls, calc_doc: Calculation) -> Self:
" in {calc_doc.dir_name}"
)

with open(convergence_file) as f:
convergence_data = json.load(f)
with open(convergence_file) as file:
convergence_data = json.load(file)

return cls(
structure=calc_doc.output.structure,
Expand Down
4 changes: 2 additions & 2 deletions src/atomate2/amset/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ def from_directory(
}
if include_mesh:
# remove duplicated data
for k in ("doping", "temperatures", "fermi_levels", "structure"):
mesh.pop(k)
for key in ("doping", "temperatures", "fermi_levels", "structure"):
mesh.pop(key)

mesh_kwargs["mesh"] = cast_dict_list(mesh)

Expand Down
14 changes: 7 additions & 7 deletions src/atomate2/cli/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def vasp_test_data(test_dir: str | Path, additional_file: list[str]) -> None:
[f" {v} -> {k}" for k, v in original_mapping.items()]
)

run_vasp_kwargs = {k: {"incar_settings": ["NSW", "ISMEAR"]} for k in mapping}
run_vasp_kwargs = {key: {"incar_settings": ["NSW", "ISMEAR"]} for key in mapping}
run_vasp_kwargs_str = pformat(run_vasp_kwargs).replace("\n", "\n ")

test_function_str = f"""Test files generated in test_data.
Expand Down Expand Up @@ -221,8 +221,8 @@ def abinit_script_maker() -> None:
"save_abinit_maker(maker)",
"",
]
with open(script_fname, "w") as f:
f.write("\n".join(out))
with open(script_fname, "w") as file:
file.write("\n".join(out))


@dev.command(context_settings={"help_option_names": ["-h", "--help"]})
Expand Down Expand Up @@ -593,8 +593,8 @@ def save_abinit_maker(maker: Maker) -> None:

caller_frame = inspect.stack()[1]
caller_filename_full = caller_frame.filename
with open(caller_filename_full) as f:
script_str = f.read()
with open(caller_filename_full) as file:
script_str = file.read()
git = shutil.which("git")
author = None
author_mail = None
Expand Down Expand Up @@ -627,7 +627,7 @@ def save_abinit_maker(maker: Maker) -> None:
"You may want to manually set it in the 'maker.json' file.",
stacklevel=2,
)
with open("maker.json", "w") as f:
with open("maker.json", "w") as file:
json.dump(
{
"author": author,
Expand All @@ -636,7 +636,7 @@ def save_abinit_maker(maker: Maker) -> None:
"maker": jsanitize(maker.as_dict()),
"script": script_str,
},
f,
file,
)


Expand Down
6 changes: 4 additions & 2 deletions src/atomate2/cp2k/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,10 @@ def copy_cp2k_outputs(
# rename files to remove relax extension
if relax_ext:
files_to_rename = {
k.name.replace(".gz", ""): k.name.replace(relax_ext, "").replace(".gz", "")
for k in all_files
file.name.replace(".gz", ""): file.name.replace(relax_ext, "").replace(
".gz", ""
)
for file in all_files
}
rename_files(files_to_rename, allow_missing=True, file_client=file_client)

Expand Down
6 changes: 3 additions & 3 deletions src/atomate2/cp2k/schemas/calculation.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ class CalculationInput(BaseModel):
@classmethod
def remove_unnecessary(cls, atomic_kind_info: dict) -> dict:
"""Remove unnecessary entry from atomic_kind_info."""
for k in atomic_kind_info:
if "total_pseudopotential_energy" in atomic_kind_info[k]:
del atomic_kind_info[k]["total_pseudopotential_energy"]
for key in atomic_kind_info:
if "total_pseudopotential_energy" in atomic_kind_info[key]:
del atomic_kind_info[key]["total_pseudopotential_energy"]
return atomic_kind_info

@field_validator("dft", mode="before")
Expand Down
6 changes: 4 additions & 2 deletions src/atomate2/qchem/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ def copy_qchem_outputs(
if opt_ext:
all_files = required_files
files_to_rename = {
k.name.replace(".gz", ""): k.name.replace(opt_ext, "").replace(".gz", "")
for k in all_files
file.name.replace(".gz", ""): file.name.replace(opt_ext, "").replace(
".gz", ""
)
for file in all_files
}
rename_files(files_to_rename, allow_missing=True, file_client=file_client)

Expand Down
10 changes: 5 additions & 5 deletions src/atomate2/utils/file_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ def link(
"""
try:
os.symlink(src_filename, dest_filename)
except OSError as e:
if e.errno == errno.EEXIST:
except OSError as exc:
if exc.errno == errno.EEXIST:
os.remove(dest_filename)
os.symlink(src_filename, dest_filename)
else:
Expand Down Expand Up @@ -546,9 +546,9 @@ def get_ssh_connection(
ssh_config = paramiko.SSHConfig().from_path(str(config_filename))

host_config = ssh_config.lookup(hostname) # type: ignore[attr-defined]
for k in ("hostname", "user", "port"):
if k in host_config:
config[k.replace("user", "username")] = host_config[k]
for key in ("hostname", "user", "port"):
if key in host_config:
config[key.replace("user", "username")] = host_config[key]

if "proxycommand" in host_config:
config["sock"] = paramiko.ProxyCommand(host_config["proxycommand"])
Expand Down
6 changes: 4 additions & 2 deletions src/atomate2/vasp/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,10 @@ def copy_vasp_outputs(
if relax_ext:
all_files = optional_files + required_files
files_to_rename = {
k.name.replace(".gz", ""): k.name.replace(relax_ext, "").replace(".gz", "")
for k in all_files
file.name.replace(".gz", ""): file.name.replace(relax_ext, "").replace(
".gz", ""
)
for file in all_files
}
rename_files(files_to_rename, allow_missing=True, file_client=file_client)

Expand Down
4 changes: 2 additions & 2 deletions tests/abinit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ def check_run_abi(ref_path: str | Path):

user = AbinitInputFile.from_file("run.abi")
assert user.ndtset == 1, f"'run.abi' has multiple datasets (ndtset={user.ndtset})."
with zopen(ref_path / "inputs" / "run.abi.gz") as f:
ref_str = f.read()
with zopen(ref_path / "inputs" / "run.abi.gz") as file:
ref_str = file.read()
ref = AbinitInputFile.from_string(ref_str.decode("utf-8"))
# Ignore the pseudos as the directory depends on the pseudo root directory
diffs = user.get_differences(ref, ignore_vars=["pseudos"])
Expand Down
12 changes: 6 additions & 6 deletions tests/abinit/sets/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ def test_abinit_input_set_write_input(abinit_test_dir):
assert os.path.isdir("testdir/tmpdata")
assert "run.abi" in dirlist
assert "abinit_input.json" in dirlist
with open("testdir/run.abi") as f:
abistr = f.read()
with open("testdir/run.abi") as file:
abistr = file.read()
assert "ecut" in abistr
with open("testdir/abinit_input.json") as f:
abijsonstr = f.read()
with open("testdir/abinit_input.json") as file:
abijsonstr = file.read()
assert "@module" in abijsonstr
with pytest.raises(FileExistsError):
ais.write_input("testdir", overwrite=False)
Expand Down Expand Up @@ -132,8 +132,8 @@ def test_abinit_input_set_write_input(abinit_test_dir):
assert os.path.exists(in_den)
assert os.path.isfile(in_den)
assert not os.path.islink(in_den)
with open("testdir/run.abi") as f:
abistr = f.read()
with open("testdir/run.abi") as file:
abistr = file.read()
assert "irdden 1" in abistr
del ais.abinit_input["irdden"]

Expand Down
12 changes: 6 additions & 6 deletions tests/abinit/sets/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def test_static_generator_get_input_set(si_structure):
# assert len(dirlist) == 6
# assert "abinit_input_set_generator.json" in dirlist
# assert "abinit_input.json" in dirlist
# with open("first_run/run.abi", "r") as f:
# runabi1_str = f.read()
# with open("first_run/run.abi") as file:
# runabi1_str = file.read()
# assert "ecut 8.5" in runabi1_str
# assert "nband 13" in runabi1_str
# assert "occopt 5" in runabi1_str
Expand All @@ -76,8 +76,8 @@ def test_static_generator_get_input_set(si_structure):
# )
# assert os.path.islink(indenpath)
# assert os.readlink(indenpath) == outdenpath
# with open("second_run/run.abi", "r") as f:
# runabi2_str = f.read()
# with open("second_run/run.abi") as file:
# runabi2_str = file.read()
# assert "ecut 8.5" in runabi2_str
# assert "nband 8" in runabi2_str
# assert "occopt" not in runabi2_str
Expand All @@ -101,8 +101,8 @@ def test_static_generator_get_input_set(si_structure):
# )
# assert os.path.islink(inwfkpath)
# assert os.readlink(inwfkpath) == outwfkpath
# with open("second_run_bis/run.abi", "r") as f:
# runabi3_str = f.read()
# with open("second_run_bis/run.abi") as file:
# runabi3_str = file.read()
# assert "ecut 8.5" in runabi3_str
# assert "nband 13" in runabi3_str
# assert "occopt 5" in runabi3_str
Expand Down
13 changes: 8 additions & 5 deletions tests/forcefields/test_md.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ def test_nve_and_dynamics_obj(si_structure: Structure, test_dir: Path):
# vs. as an ase.md.md.MolecularDynamics object

output = {}
for k in ["from_str", "from_dyn"]:
if k == "from_str":
for attr in ("from_str", "from_dyn"):
if attr == "from_str":
dyn = "velocityverlet"
elif k == "from_dyn":
elif attr == "from_dyn":
dyn = VelocityVerlet

job = CHGNetMDMaker(
Expand All @@ -180,7 +180,7 @@ def test_nve_and_dynamics_obj(si_structure: Structure, test_dir: Path):
).make(si_structure)

response = run_locally(job, ensure_success=True)
output[k] = response[next(iter(response))][1].output
output[attr] = response[next(iter(response))][1].output

# check that energy and volume are constants
assert output["from_str"].output.energy == pytest.approx(-10.6, abs=0.1)
Expand All @@ -195,7 +195,10 @@ def test_nve_and_dynamics_obj(si_structure: Structure, test_dir: Path):
# ensure that output is consistent if molecular dynamics object is specified
# as str or as MolecularDynamics object
for attr in ("energy", "forces", "stress", "structure"):
vals = {k: getattr(output[k].output, attr) for k in ("from_str", "from_dyn")}
vals = {
attr: getattr(output[attr].output, attr)
for attr in ("from_str", "from_dyn")
}
if isinstance(vals["from_str"], float):
assert vals["from_str"] == pytest.approx(vals["from_dyn"])
elif isinstance(vals["from_str"], Structure):
Expand Down
23 changes: 8 additions & 15 deletions tests/vasp/flows/test_defect.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_ccd_maker(mock_vasp, clean_dir, test_dir):
"finite diff q1": "Si_config_coord/finite_diff_q1",
"finite diff q2": "Si_config_coord/finite_diff_q2",
}
fake_run_vasp_kwargs = {k: {"incar_settings": ["ISIF"]} for k in ref_paths}
fake_run_vasp_kwargs = {path: {"incar_settings": ["ISIF"]} for path in ref_paths}

# automatically use fake VASP and write POTCAR.spec during the test
mock_vasp(ref_paths, fake_run_vasp_kwargs)
Expand Down Expand Up @@ -82,7 +82,7 @@ def test_nonrad_maker(mock_vasp, clean_dir, test_dir, monkeypatch):
"finite diff q1": "Si_config_coord/finite_diff_q1",
"finite diff q2": "Si_config_coord/finite_diff_q2",
}
fake_run_vasp_kwargs = {k: {"incar_settings": ["ISIF"]} for k in ref_paths}
fake_run_vasp_kwargs = {path: {"incar_settings": ["ISIF"]} for path in ref_paths}

# automatically use fake VASP and write POTCAR.spec during the test
mock_vasp(ref_paths, fake_run_vasp_kwargs)
Expand Down Expand Up @@ -140,7 +140,8 @@ def test_formation_energy_maker(mock_vasp, clean_dir, test_dir, monkeypatch):
}

fake_run_vasp_kwargs = {
k: {"incar_settings": ["ISIF"], "check_inputs": ["incar"]} for k in ref_paths
path: {"incar_settings": ["ISIF"], "check_inputs": ["incar"]}
for path in ref_paths
}

# automatically use fake VASP and write POTCAR.spec during the test
Expand Down Expand Up @@ -177,18 +178,10 @@ def _check_plnr_locpot(name):
plnr_locpot = job["output"]["calcs_reversed"][0]["output"]["locpot"]
assert set(plnr_locpot) == {"0", "1", "2"}

for k in ref_paths:
_check_plnr_locpot(k)
for path in ref_paths:
_check_plnr_locpot(path)

# make sure the the you can restart the calculation from prv
prv_dir = test_dir / "vasp/GaN_Mg_defect/bulk_relax/outputs"
flow2 = maker.make(
defects[0],
bulk_supercell_dir=prv_dir,
defect_index=0,
)
_ = run_locally(
flow2,
create_folders=True,
ensure_success=True,
)
flow2 = maker.make(defects[0], bulk_supercell_dir=prv_dir, defect_index=0)
_ = run_locally(flow2, create_folders=True, ensure_success=True)
9 changes: 4 additions & 5 deletions tests/vasp/jobs/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@


def test_static_maker(mock_vasp, clean_dir, si_structure):
jstore = jobflow.SETTINGS.JOB_STORE
job_store = jobflow.SETTINGS.JOB_STORE

# mapping from job name to directory containing test files
ref_paths = {"static": "Si_band_structure/static"}
Expand All @@ -42,10 +42,9 @@ def test_static_maker(mock_vasp, clean_dir, si_structure):
assert isinstance(output1, TaskDoc)
assert output1.output.energy == approx(-10.85037078)

with jstore.additional_stores["data"] as s:
doc = s.query_one({"job_uuid": job.uuid})
dd = doc["data"]
assert dd["@class"] == "Chgcar"
with job_store.additional_stores["data"] as store:
doc = store.query_one({"job_uuid": job.uuid})
assert doc["data"]["@class"] == "Chgcar"


def test_relax_maker(mock_vasp, clean_dir, si_structure):
Expand Down

0 comments on commit 90518e0

Please sign in to comment.