Skip to content

Commit

Permalink
define skip_if_no_openbabel decorator once and reuse
Browse files Browse the repository at this point in the history
  • Loading branch information
janosh committed Mar 12, 2024
1 parent 26b828b commit 266cc0f
Show file tree
Hide file tree
Showing 13 changed files with 54 additions and 51 deletions.
4 changes: 2 additions & 2 deletions custodian/ansible/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions custodian/cp2k/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions custodian/custodian.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions custodian/feff/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions custodian/lobster/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions custodian/qchem/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 6 additions & 6 deletions custodian/vasp/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions custodian/vasp/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
16 changes: 8 additions & 8 deletions tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion tests/qchem/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion tests/qchem/test_job_handler_interaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 16 additions & 15 deletions tests/qchem/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions tests/vasp/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")

Expand Down

0 comments on commit 266cc0f

Please sign in to comment.