From 266cc0faf42f13699a82cf43328830712ecc63eb Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Tue, 12 Mar 2024 06:02:37 +0100 Subject: [PATCH] define skip_if_no_openbabel decorator once and reuse --- custodian/ansible/actions.py | 4 +-- custodian/cp2k/utils.py | 4 +-- custodian/custodian.py | 8 +++--- custodian/feff/handlers.py | 4 +-- custodian/lobster/handlers.py | 4 +-- custodian/qchem/jobs.py | 4 +-- custodian/vasp/handlers.py | 12 ++++---- custodian/vasp/jobs.py | 4 +-- tasks.py | 16 +++++------ tests/qchem/test_handlers.py | 3 +- tests/qchem/test_job_handler_interaction.py | 3 +- tests/qchem/test_jobs.py | 31 +++++++++++---------- tests/vasp/test_handlers.py | 8 +++--- 13 files changed, 54 insertions(+), 51 deletions(-) diff --git a/custodian/ansible/actions.py b/custodian/ansible/actions.py index a975fb53..91176984 100644 --- a/custodian/ansible/actions.py +++ b/custodian/ansible/actions.py @@ -231,8 +231,8 @@ def file_create(filename, settings, directory): raise ValueError("Settings must only contain one item with key 'content'.") for k, v in settings.items(): if k == "content": - with open(filename, "w") as f: - f.write(v) + with open(filename, "w") as file: + file.write(v) @staticmethod def file_move(filename, settings, directory): diff --git a/custodian/cp2k/utils.py b/custodian/cp2k/utils.py index 9d0a99a5..d1512060 100644 --- a/custodian/cp2k/utils.py +++ b/custodian/cp2k/utils.py @@ -176,8 +176,8 @@ def can_use_ot(output, ci, minimum_band_gap=0.1): def tail(filename, n=10): """Returns the last n lines of a file as a list (including empty lines).""" - with open(filename) as f: - t = deque(f, n) + with open(filename) as file: + t = deque(file, n) if t: return t return [""] * n diff --git a/custodian/custodian.py b/custodian/custodian.py index 05ab6757..5693ab97 100644 --- a/custodian/custodian.py +++ b/custodian/custodian.py @@ -215,7 +215,7 @@ def _load_checkpoint(directory): chkpt = sorted(chkpts, key=lambda c: int(c.split(".")[-3]))[0] restart = int(chkpt.split(".")[-3]) logger.info(f"Loading from checkpoint file {chkpt}...") - with tarfile.open(chkpt) as t: + with tarfile.open(chkpt) as file: def is_within_directory(directory, target): abs_directory = os.path.abspath(directory) @@ -233,7 +233,7 @@ def safe_extract(tar, path=directory, members=None, *, numeric_owner=False): tar.extractall(path, members, numeric_owner=numeric_owner) - safe_extract(t, directory) + safe_extract(file, directory) # Log the corrections to a json file. run_log = loadfn(os.path.join(directory, Custodian.LOG_FILE), cls=MontyDecoder) @@ -249,8 +249,8 @@ def _save_checkpoint(directory, index): try: Custodian._delete_checkpoints(directory) n = os.path.join(directory, f"custodian.chk.{index}.tar.gz") - with tarfile.open(n, mode="w:gz", compresslevel=3) as f: - f.add(directory, arcname=".") + with tarfile.open(n, mode="w:gz", compresslevel=3) as file: + file.add(directory, arcname=".") logger.info(f"Checkpoint written to {n}") except Exception: logger.info("Checkpointing failed") diff --git a/custodian/feff/handlers.py b/custodian/feff/handlers.py index 896c06c3..2c54d75a 100644 --- a/custodian/feff/handlers.py +++ b/custodian/feff/handlers.py @@ -56,8 +56,8 @@ def _notconverge_check(self, directory): # Process the output file and get converge information not_converge_pattern = re.compile("Convergence not reached.*") converge_pattern = re.compile("Convergence reached.*") - with open(os.path.join(directory, self.output_filename)) as f: - for line in f: + with open(os.path.join(directory, self.output_filename)) as file: + for line in file: if len(not_converge_pattern.findall(line)) > 0: return True diff --git a/custodian/lobster/handlers.py b/custodian/lobster/handlers.py index 20aa9f39..bba3c8c9 100644 --- a/custodian/lobster/handlers.py +++ b/custodian/lobster/handlers.py @@ -33,8 +33,8 @@ def check(self, directory: str = "./") -> bool: """ # checks if correct number of bands is available try: - with open(os.path.join(directory, self.output_filename)) as f: - data = f.read() + with open(os.path.join(directory, self.output_filename)) as file: + data = file.read() return "You are employing too few bands in your PAW calculation." in data except OSError: return False diff --git a/custodian/qchem/jobs.py b/custodian/qchem/jobs.py index 399310f9..c2b9001f 100644 --- a/custodian/qchem/jobs.py +++ b/custodian/qchem/jobs.py @@ -191,8 +191,8 @@ def run(self, directory: str | Path = "./"): if os.path.exists(os.path.join(os.environ["QCSCRATCH"], "53.0")): os.makedirs(local_scratch, exist_ok=True) shutil.move(os.path.join(os.environ["QCSCRATCH"], "53.0"), local_scratch) - with open(self.qclog_file, "w") as qclog: - return subprocess.Popen(self.current_command, cwd=directory, stdout=qclog, shell=True) # pylint: disable=R1732 + with open(self.qclog_file, "w") as qc_log: + return subprocess.Popen(self.current_command, cwd=directory, stdout=qc_log, shell=True) # pylint: disable=R1732 @classmethod def opt_with_frequency_flattener( diff --git a/custodian/vasp/handlers.py b/custodian/vasp/handlers.py index 9115b713..71d699bf 100644 --- a/custodian/vasp/handlers.py +++ b/custodian/vasp/handlers.py @@ -709,8 +709,8 @@ def __init__(self, output_filename: str = "std_err.txt"): def check(self, directory="./"): """Check for error.""" self.errors = set() - with open(os.path.join(directory, self.output_filename)) as f: - for line in f: + with open(os.path.join(directory, self.output_filename)) as file: + for line in file: line = line.strip() for err, msgs in LrfCommutatorHandler.error_msgs.items(): for msg in msgs: @@ -825,8 +825,8 @@ def check(self, directory="./"): """Check for error.""" incar = Incar.from_file(os.path.join(directory, "INCAR")) self.errors = set() - with open(os.path.join(directory, self.output_filename)) as f: - for line in f: + with open(os.path.join(directory, self.output_filename)) as file: + for line in file: line = line.strip() for err, msgs in AliasingErrorHandler.error_msgs.items(): for msg in msgs: @@ -1009,8 +1009,8 @@ def check(self, directory="./"): return False except Exception: pass - with open(os.path.join(directory, self.output_filename)) as f: - for line in f: + with open(os.path.join(directory, self.output_filename)) as file: + for line in file: line = line.strip() if line.find(msg) != -1: return True diff --git a/custodian/vasp/jobs.py b/custodian/vasp/jobs.py index 13de45e4..636c3893 100644 --- a/custodian/vasp/jobs.py +++ b/custodian/vasp/jobs.py @@ -707,8 +707,8 @@ def terminate(self, directory="./"): if (vasprun_path in open_paths) and psutil.pid_exists(proc.pid): proc.kill() return - except (psutil.NoSuchProcess, psutil.AccessDenied) as e: - logger.warning(f"Exception {e} encountered while killing VASP.") + except (psutil.NoSuchProcess, psutil.AccessDenied) as exc: + logger.warning(f"Exception {exc} encountered while killing VASP.") continue logger.warning( diff --git a/tasks.py b/tasks.py index baac0c13..72849949 100644 --- a/tasks.py +++ b/tasks.py @@ -27,14 +27,14 @@ def make_doc(ctx): ctx.run("cp markdown/custodian*.md .") ctx.run("rm custodian*tests*.md", warn=True) for fn in glob("custodian*.md"): - with open(fn) as f: - lines = [line.rstrip() for line in f if "Submodules" not in line] + with open(fn) as file: + lines = [line.rstrip() for line in file if "Submodules" not in line] if fn == "custodian.md": preamble = ["---", "layout: default", "title: API Documentation", "nav_order: 6", "---", ""] else: preamble = ["---", "layout: default", "title: " + fn, "nav_exclude: true", "---", ""] - with open(fn, "w") as f: - f.write("\n".join(preamble + lines)) + with open(fn, "w") as file: + file.write("\n".join(preamble + lines)) ctx.run("rm -r markdown doctrees", warn=True) @@ -117,16 +117,16 @@ def update_changelog(ctx, version=None, sim=False): pass else: misc.append("- " + line) - with open("docs/changelog.md") as f: - contents = f.read() + with open("docs/changelog.md") as file: + contents = file.read() head = "# Change Log" i = contents.find(head) i += len(head) contents = contents[0:i] + f"\n\n## {NEW_VER}\n" + "\n".join(lines) + contents[i:] if not sim: - with open("docs/changelog.md", "w") as f: - f.write(contents) + with open("docs/changelog.md", "w") as file: + file.write(contents) ctx.run("open docs/changelog.md") else: print(contents) diff --git a/tests/qchem/test_handlers.py b/tests/qchem/test_handlers.py index ae0b86b8..9133730c 100644 --- a/tests/qchem/test_handlers.py +++ b/tests/qchem/test_handlers.py @@ -26,9 +26,10 @@ TEST_DIR = f"{TEST_FILES}/qchem/new_test_files" SCR_DIR = f"{TEST_DIR}/scratch" CWD = os.getcwd() +skip_if_no_openbabel = unittest.skipIf(ob is None, "openbabel not installed") -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class QChemErrorHandlerTest(TestCase): def setUp(self): os.makedirs(SCR_DIR) diff --git a/tests/qchem/test_job_handler_interaction.py b/tests/qchem/test_job_handler_interaction.py index d54bbd61..083484a5 100644 --- a/tests/qchem/test_job_handler_interaction.py +++ b/tests/qchem/test_job_handler_interaction.py @@ -25,9 +25,10 @@ TEST_DIR = f"{TEST_FILES}/qchem/new_test_files" SCR_DIR = f"{TEST_DIR}/scratch" CWD = os.getcwd() +skip_if_no_openbabel = unittest.skipIf(ob is None, "openbabel not installed") -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class FFOptJobHandlerInteraction(TestCase): def _check_equivalent_inputs(self, input1, input2): QCinput1 = QCInput.from_file(input1) diff --git a/tests/qchem/test_jobs.py b/tests/qchem/test_jobs.py index 7144ce30..92964023 100644 --- a/tests/qchem/test_jobs.py +++ b/tests/qchem/test_jobs.py @@ -27,9 +27,10 @@ TEST_DIR = f"{TEST_FILES}/qchem/new_test_files" SCR_DIR = f"{TEST_DIR}/scratch" CWD = os.getcwd() +skip_if_no_openbabel = unittest.skipIf(ob is None, "openbabel not installed") -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class QCJobTest(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -88,7 +89,7 @@ def test_save_scratch(self): assert os.environ["QCLOCALSCR"] == "/tmp/scratch" -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFComplexUnlinkedTest(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -151,7 +152,7 @@ def test_OptFF(self): ) -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTestComplexLinkedChangeNsegTest(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -236,7 +237,7 @@ def test_OptFF(self): ) -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTest(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -317,7 +318,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTest1(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -347,7 +348,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTest2(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -397,7 +398,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTestSwitching(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -509,7 +510,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTest6004(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -619,7 +620,7 @@ def test_OptFF(self): ) -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTest5952(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -669,7 +670,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFTest5690(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -781,7 +782,7 @@ def test_OptFF(self): next(job) -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFSmallNegFreqTest(TestCase): def setUp(self): os.makedirs(f"{SCR_DIR}/scratch", exist_ok=True) @@ -917,7 +918,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class OptFFSingleFreqFragsTest(TestCase): def setUp(self): os.makedirs(f"{SCR_DIR}/scratch", exist_ok=True) @@ -973,7 +974,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class TSFFTest(TestCase): def setUp(self): os.makedirs(SCR_DIR) @@ -1024,7 +1025,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class TSFFFreqfirstTest(TestCase): def setUp(self): os.makedirs(f"{SCR_DIR}/scratch", exist_ok=True) @@ -1096,7 +1097,7 @@ def test_OptFF(self): job.__next__() -@unittest.skipIf(ob is None, "openbabel not installed") +@skip_if_no_openbabel class TSFFFreqFirstMultipleCyclesTest(TestCase): def setUp(self): os.makedirs(f"{SCR_DIR}/scratch", exist_ok=True) diff --git a/tests/vasp/test_handlers.py b/tests/vasp/test_handlers.py index a7c0fa5d..5698aa20 100644 --- a/tests/vasp/test_handlers.py +++ b/tests/vasp/test_handlers.py @@ -933,8 +933,8 @@ def test_check_and_correct(self): # Test that the STOPCAR is written correctly. handler.correct() - with open("STOPCAR") as f: - content = f.read() + with open("STOPCAR") as file: + content = file.read() assert content == "LSTOP = .TRUE." os.remove("STOPCAR") @@ -945,8 +945,8 @@ def test_check_and_correct(self): assert handler.check() handler.correct() - with open("STOPCAR") as f: - content = f.read() + with open("STOPCAR") as file: + content = file.read() assert content == "LABORT = .TRUE." os.remove("STOPCAR")