diff --git a/fsqc/checkMotion.py b/fsqc/checkMotion.py index 012bed7..eae4031 100644 --- a/fsqc/checkMotion.py +++ b/fsqc/checkMotion.py @@ -593,6 +593,8 @@ def checkMotion( import numpy as np from scipy import ndimage as ndimg + from fsqc.fsqcUtils import ensureDir + # mriqc pulls in nipype/niworkflows, which are known to attach their own # handler(s) to the root logger as a side effect of import -- undoing # fsqc's own _start_logging() setup and causing every subsequent @@ -632,7 +634,7 @@ def _nan_metrics_dict(): logging.info("Computing MRIQC-style motion/noise metrics ...") if write_masks and output_dir is not None: - os.makedirs(output_dir, exist_ok=True) + ensureDir(output_dir) # resolve + conform the reference volume (single grid, shared with # aseg/aparc below); bail out with NaNs if it doesn't exist diff --git a/fsqc/fsqcMain.py b/fsqc/fsqcMain.py index d5a51d4..3c32eb3 100644 --- a/fsqc/fsqcMain.py +++ b/fsqc/fsqcMain.py @@ -901,6 +901,8 @@ def _check_arguments(argsDict): import tempfile import warnings + from fsqc.fsqcUtils import ensureDir + logging.captureWarnings(True) # -------------------------------------------------------------------------- @@ -966,7 +968,7 @@ def _check_arguments(argsDict): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "screenshots")) + ensureDir(os.path.join(argsDict["output_dir"], "screenshots")) except Exception as e: logging.error( "ERROR: cannot create screenshots directory " @@ -1070,7 +1072,7 @@ def _check_arguments(argsDict): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "skullstrip")) + ensureDir(os.path.join(argsDict["output_dir"], "skullstrip")) except Exception as e: logging.error( "ERROR: cannot create skullstrip directory " @@ -1102,7 +1104,7 @@ def _check_arguments(argsDict): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "fornix")) + ensureDir(os.path.join(argsDict["output_dir"], "fornix")) except Exception as e: logging.error( "ERROR: cannot create fornix directory " @@ -1134,7 +1136,7 @@ def _check_arguments(argsDict): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "hypothalamus")) + ensureDir(os.path.join(argsDict["output_dir"], "hypothalamus")) except Exception as e: logging.error( "ERROR: cannot create hypothalamus directory " @@ -1166,7 +1168,7 @@ def _check_arguments(argsDict): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "hippocampus")) + ensureDir(os.path.join(argsDict["output_dir"], "hippocampus")) except Exception as e: logging.error( "ERROR: cannot create hippocampus directory " @@ -1210,7 +1212,7 @@ def _check_arguments(argsDict): ) else: try: - os.makedirs(os.path.join(argsDict["output_dir"], "brainprint")) + ensureDir(os.path.join(argsDict["output_dir"], "brainprint")) except Exception as e: logging.error( "ERROR: cannot create brainprint directory " @@ -1242,7 +1244,7 @@ def _check_arguments(argsDict): ) else: try: - os.makedirs(os.path.join(argsDict["output_dir"], "outliers")) + ensureDir(os.path.join(argsDict["output_dir"], "outliers")) except Exception as e: logging.error( "ERROR: cannot create outliers directory " @@ -1661,6 +1663,7 @@ def _do_fsqc(argsDict): from fsqc.evaluateFornixSegmentation import evaluateFornixSegmentation from fsqc.evaluateHippocampalSegmentation import evaluateHippocampalSegmentation from fsqc.evaluateHypothalamicSegmentation import evaluateHypothalamicSegmentation + from fsqc.fsqcUtils import ensureDir from fsqc.outlierDetection import outlierDetection, outlierTable # ------------------------------------------------------------------------------ @@ -1743,8 +1746,7 @@ def _do_fsqc(argsDict): # check / create subject-specific status_outdir status_outdir = os.path.join(argsDict["output_dir"], "status", subject) - if not os.path.isdir(status_outdir): - os.makedirs(status_outdir) + ensureDir(status_outdir) # if it already exists, read statusfile status_dict = dict() @@ -1783,8 +1785,7 @@ def _do_fsqc(argsDict): # check / create subject-specific metrics_outdir metrics_outdir = os.path.join(argsDict["output_dir"], "metrics", subject) - if not os.path.isdir(metrics_outdir): - os.makedirs(metrics_outdir) + ensureDir(metrics_outdir) # metrics_status = 0 @@ -1998,7 +1999,9 @@ def _do_fsqc(argsDict): } ) - # write to file + # write to file; the output directory was created before the + # metrics were computed, so make sure that it is still visible + ensureDir(metrics_outdir) pd.DataFrame(metricsDict[subject], index=[subject]).to_csv( os.path.join( argsDict["output_dir"], "metrics", subject, "metrics.csv" @@ -2146,8 +2149,7 @@ def _do_fsqc(argsDict): screenshots_outdir = os.path.join( argsDict["output_dir"], "screenshots", subject ) - if not os.path.isdir(screenshots_outdir): - os.makedirs(screenshots_outdir) + ensureDir(screenshots_outdir) outfile = os.path.join(screenshots_outdir, subject + ".png") # @@ -2370,8 +2372,7 @@ def _do_fsqc(argsDict): surfaces_outdir = os.path.join( argsDict["output_dir"], "surfaces", subject ) - if not os.path.isdir(surfaces_outdir): - os.makedirs(surfaces_outdir) + ensureDir(surfaces_outdir) # if surfaces_status == 0: @@ -2448,8 +2449,7 @@ def _do_fsqc(argsDict): skullstrip_outdir = os.path.join( argsDict["output_dir"], "skullstrip", subject ) - if not os.path.isdir(skullstrip_outdir): - os.makedirs(skullstrip_outdir) + ensureDir(skullstrip_outdir) outfile = os.path.join(skullstrip_outdir, subject + ".png") # @@ -2578,8 +2578,7 @@ def _do_fsqc(argsDict): # check / create subject-specific fornix_outdir fornix_outdir = os.path.join(argsDict["output_dir"], "fornix", subject) - if not os.path.isdir(fornix_outdir): - os.makedirs(fornix_outdir) + ensureDir(fornix_outdir) fornix_screenshot_outfile = os.path.join(fornix_outdir, "cc.png") # @@ -2716,8 +2715,7 @@ def _do_fsqc(argsDict): hypothalamus_outdir = os.path.join( argsDict["output_dir"], "hypothalamus", subject ) - if not os.path.isdir(hypothalamus_outdir): - os.makedirs(hypothalamus_outdir) + ensureDir(hypothalamus_outdir) hypothalamus_screenshot_outfile = os.path.join( hypothalamus_outdir, "hypothalamus.png" ) @@ -2801,8 +2799,7 @@ def _do_fsqc(argsDict): hippocampus_outdir = os.path.join( argsDict["output_dir"], "hippocampus", subject ) - if not os.path.isdir(hippocampus_outdir): - os.makedirs(hippocampus_outdir) + ensureDir(hippocampus_outdir) hippocampus_screenshot_outfile_left = os.path.join( hippocampus_outdir, "hippocampus-left.png" ) @@ -2880,6 +2877,9 @@ def _do_fsqc(argsDict): # 1: Failed # 2: Not done # 3: Skipped + # the output directory was created at the beginning of the subject + # loop, so make sure that it is still visible + ensureDir(status_outdir) pd.DataFrame(statusDict[subject], index=[subject]).T.to_csv( os.path.join(argsDict["output_dir"], "status", subject, "status.txt"), header=False, @@ -3627,6 +3627,8 @@ def _start_logging(argsDict): import time import traceback + from fsqc.fsqcUtils import ensureDir + # setup function to log uncaught exceptions def foo(exctype, value, tb): # log @@ -3652,7 +3654,7 @@ def foo(exctype, value, tb): logging.info("Found output directory " + argsDict["output_dir"]) else: try: - os.mkdir(argsDict["output_dir"]) + ensureDir(argsDict["output_dir"]) except Exception as e: logging.error( "ERROR: cannot create output directory " + argsDict["output_dir"] @@ -3667,7 +3669,7 @@ def foo(exctype, value, tb): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "status")) + ensureDir(os.path.join(argsDict["output_dir"], "status")) except Exception as e: logging.error( "ERROR: cannot create status directory " @@ -3683,7 +3685,7 @@ def foo(exctype, value, tb): ) else: try: - os.mkdir(os.path.join(argsDict["output_dir"], "metrics")) + ensureDir(os.path.join(argsDict["output_dir"], "metrics")) except Exception as e: logging.error( "ERROR: cannot create metrics directory " diff --git a/fsqc/fsqcUtils.py b/fsqc/fsqcUtils.py index 254b894..e8edec8 100644 --- a/fsqc/fsqcUtils.py +++ b/fsqc/fsqcUtils.py @@ -7,6 +7,58 @@ # ------------------------------------------------------------------------------ +def ensureDir(path, retries=3, delay=0.5): + """ + Create a directory, including any missing parent directories. + + Unlike a plain 'os.makedirs(path, exist_ok=True)', this function also + tolerates filesystems that report directory metadata with a delay. + + Parameters + ---------- + path : str + Path of the directory to create. + retries : int, default: 3 + Number of additional attempts before giving up. + delay : float, default: 0.5 + Seconds to wait between attempts. + + Returns + ------- + None + This function returns nothing. + + Notes + ----- + 'os.makedirs()' swallows the 'FileExistsError' raised by its underlying + 'mkdir()' call only if a subsequent 'os.path.isdir()' check confirms that + the existing path is a directory; otherwise the error is re-raised, even + with 'exist_ok=True'. On networked filesystems (SMB/CIFS, NFS) that check + may transiently fail for a directory that does exist, because the client + still serves a cached, outdated view of the parent directory. Retrying + after a short delay gives the cache time to refresh. + + Note that a genuine error is still raised once the attempts are exhausted, + and that a non-directory file blocking the path is never removed. + """ + import os + import time + + for attempt in range(retries + 1): + try: + os.makedirs(path, exist_ok=True) + return + except FileExistsError: + # 'path' exists, but was not recognized as a directory; give a + # possibly outdated cache time to refresh before trying again. + if attempt == retries: + raise + time.sleep(delay) + + +# ------------------------------------------------------------------------------ + + def importMGH(filename): """ A function to read Freesurfer MGH files. diff --git a/fsqc/utils/tests/test_ensure_dir.py b/fsqc/utils/tests/test_ensure_dir.py new file mode 100644 index 0000000..b678972 --- /dev/null +++ b/fsqc/utils/tests/test_ensure_dir.py @@ -0,0 +1,65 @@ +import os +import time + +import pytest + +from fsqc.fsqcUtils import ensureDir + + +def test_ensure_dir_creates_missing_directories(tmp_path): + """Test that missing directories, including parents, are created.""" + target = tmp_path / "metrics" / "subject-01" + assert not target.is_dir() + + ensureDir(str(target)) + + assert target.is_dir() + + +def test_ensure_dir_accepts_existing_directory(tmp_path): + """Test that an existing directory is left alone.""" + target = tmp_path / "metrics" + target.mkdir() + (target / "metrics.csv").write_text("subject\n") + + ensureDir(str(target)) + + assert target.is_dir() + assert (target / "metrics.csv").read_text() == "subject\n" + + +def test_ensure_dir_recovers_from_outdated_metadata(tmp_path, monkeypatch): + """Test that a FileExistsError for an existing directory is tolerated.""" + target = tmp_path / "metrics" + target.mkdir() + + calls = [] + makedirs = os.makedirs + + def flaky_makedirs(path, *args, **kwargs): + # emulate a filesystem whose outdated cache makes the os.path.isdir() + # check within os.makedirs() fail for a directory that does exist + calls.append(path) + if len(calls) == 1: + raise FileExistsError(17, "File exists", str(path)) + return makedirs(path, *args, **kwargs) + + monkeypatch.setattr(os, "makedirs", flaky_makedirs) + monkeypatch.setattr(time, "sleep", lambda seconds: None) + + ensureDir(str(target)) + + assert calls == [str(target), str(target)] + assert target.is_dir() + + +def test_ensure_dir_reports_persistent_errors(tmp_path): + """Test that a file blocking the path raises and is not removed.""" + target = tmp_path / "metrics" + target.write_text("not a directory\n") + + with pytest.raises(FileExistsError): + ensureDir(str(target), retries=0) + + assert target.is_file() + assert target.read_text() == "not a directory\n"