From 90518e0d207400a36943b7b9b11dbc879a2234ba Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Tue, 14 May 2024 12:56:56 -0400 Subject: [PATCH] more descriptive variable names --- src/atomate2/aims/jobs/convergence.py | 4 ++-- src/atomate2/aims/schemas/task.py | 4 ++-- src/atomate2/amset/schemas.py | 4 ++-- src/atomate2/cli/dev.py | 14 +++++++------- src/atomate2/cp2k/files.py | 6 ++++-- src/atomate2/cp2k/schemas/calculation.py | 6 +++--- src/atomate2/qchem/files.py | 6 ++++-- src/atomate2/utils/file_client.py | 10 +++++----- src/atomate2/vasp/files.py | 6 ++++-- tests/abinit/conftest.py | 4 ++-- tests/abinit/sets/test_base.py | 12 ++++++------ tests/abinit/sets/test_core.py | 12 ++++++------ tests/forcefields/test_md.py | 13 ++++++++----- tests/vasp/flows/test_defect.py | 23 ++++++++--------------- tests/vasp/jobs/test_core.py | 9 ++++----- 15 files changed, 67 insertions(+), 66 deletions(-) diff --git a/src/atomate2/aims/jobs/convergence.py b/src/atomate2/aims/jobs/convergence.py index 87a9741b88..09ea564da2 100644 --- a/src/atomate2/aims/jobs/convergence.py +++ b/src/atomate2/aims/jobs/convergence.py @@ -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 diff --git a/src/atomate2/aims/schemas/task.py b/src/atomate2/aims/schemas/task.py index 88ef068afc..c3e537100c 100644 --- a/src/atomate2/aims/schemas/task.py +++ b/src/atomate2/aims/schemas/task.py @@ -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, diff --git a/src/atomate2/amset/schemas.py b/src/atomate2/amset/schemas.py index b36bd522e3..005cf7278d 100644 --- a/src/atomate2/amset/schemas.py +++ b/src/atomate2/amset/schemas.py @@ -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) diff --git a/src/atomate2/cli/dev.py b/src/atomate2/cli/dev.py index f0d66a6a15..47708856ce 100644 --- a/src/atomate2/cli/dev.py +++ b/src/atomate2/cli/dev.py @@ -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. @@ -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"]}) @@ -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 @@ -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, @@ -636,7 +636,7 @@ def save_abinit_maker(maker: Maker) -> None: "maker": jsanitize(maker.as_dict()), "script": script_str, }, - f, + file, ) diff --git a/src/atomate2/cp2k/files.py b/src/atomate2/cp2k/files.py index 52b2fb2cb9..6801ad3436 100644 --- a/src/atomate2/cp2k/files.py +++ b/src/atomate2/cp2k/files.py @@ -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) diff --git a/src/atomate2/cp2k/schemas/calculation.py b/src/atomate2/cp2k/schemas/calculation.py index aad343be8c..69d44731df 100644 --- a/src/atomate2/cp2k/schemas/calculation.py +++ b/src/atomate2/cp2k/schemas/calculation.py @@ -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") diff --git a/src/atomate2/qchem/files.py b/src/atomate2/qchem/files.py index 2509c1a7bc..c1116155b9 100644 --- a/src/atomate2/qchem/files.py +++ b/src/atomate2/qchem/files.py @@ -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) diff --git a/src/atomate2/utils/file_client.py b/src/atomate2/utils/file_client.py index dae6fe7b63..0e77aa4c13 100644 --- a/src/atomate2/utils/file_client.py +++ b/src/atomate2/utils/file_client.py @@ -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: @@ -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"]) diff --git a/src/atomate2/vasp/files.py b/src/atomate2/vasp/files.py index 9c85698475..70ec20cbb6 100644 --- a/src/atomate2/vasp/files.py +++ b/src/atomate2/vasp/files.py @@ -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) diff --git a/tests/abinit/conftest.py b/tests/abinit/conftest.py index e927da15ba..b431c8a389 100644 --- a/tests/abinit/conftest.py +++ b/tests/abinit/conftest.py @@ -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"]) diff --git a/tests/abinit/sets/test_base.py b/tests/abinit/sets/test_base.py index 2e5578ec35..6d60ed1aab 100644 --- a/tests/abinit/sets/test_base.py +++ b/tests/abinit/sets/test_base.py @@ -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) @@ -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"] diff --git a/tests/abinit/sets/test_core.py b/tests/abinit/sets/test_core.py index 504393883e..ef5ac7cf48 100644 --- a/tests/abinit/sets/test_core.py +++ b/tests/abinit/sets/test_core.py @@ -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 @@ -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 @@ -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 diff --git a/tests/forcefields/test_md.py b/tests/forcefields/test_md.py index 321593d804..0b1c27e57e 100644 --- a/tests/forcefields/test_md.py +++ b/tests/forcefields/test_md.py @@ -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( @@ -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) @@ -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): diff --git a/tests/vasp/flows/test_defect.py b/tests/vasp/flows/test_defect.py index c11744de51..cb18af69c2 100644 --- a/tests/vasp/flows/test_defect.py +++ b/tests/vasp/flows/test_defect.py @@ -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) @@ -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) @@ -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 @@ -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) diff --git a/tests/vasp/jobs/test_core.py b/tests/vasp/jobs/test_core.py index 633816f069..55d4bf0a73 100644 --- a/tests/vasp/jobs/test_core.py +++ b/tests/vasp/jobs/test_core.py @@ -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"} @@ -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):