Skip to content

Commit a0c4948

Browse files
committed
Updates from comments
Signed-off-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
1 parent a96b14e commit a0c4948

2 files changed

Lines changed: 26 additions & 8 deletions

File tree

monai/apps/utils.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ def download_and_extract(
433433
extractall(filepath=filename, output_dir=output_dir, file_type=file_type, has_base=has_base)
434434

435435

436-
def create_temp_dir(directory: str | None = None, delete_on_finalise: bool = False) -> str:
436+
def create_temp_dir(directory: PathLike | None = None, delete_on_finalise: bool = False) -> str:
437437
"""
438438
Creates or uses an existing temporary directory. If `directory` is given, this is used as the path to a directory
439439
which is created if it doesn't exist already. If `directory` is None, the value of the environment variable
@@ -455,6 +455,8 @@ def create_temp_dir(directory: str | None = None, delete_on_finalise: bool = Fal
455455
if directory is None:
456456
directory = tempfile.mkdtemp()
457457
delete_on_finalise = True
458+
else:
459+
directory = str(directory) # convert Path if given
458460

459461
os.makedirs(directory, exist_ok=True)
460462

tests/apps/test_create_temp_dir.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
from __future__ import annotations
1313

1414
import os
15+
import shutil
1516
import tempfile
1617
import unittest
18+
from pathlib import Path
1719
from unittest.mock import patch
1820

1921
from monai.apps import create_temp_dir
@@ -25,13 +27,15 @@
2527
class TestCreateTempDir(unittest.TestCase):
2628
def test_basic_use(self):
2729
"""Test basic usage which should create a new random temporary directory."""
28-
try:
29-
data_dir = os.environ.pop(MONAI_DATA_DIRECTORY, None) # ignore the environment variable if present
3030

31-
test_dir = create_temp_dir()
31+
data_dir = os.environ.pop(MONAI_DATA_DIRECTORY, None) # ignore the environment variable if present
32+
try:
33+
with patch("atexit.register") as mock_reg:
34+
test_dir = create_temp_dir()
3235

33-
self.assertTrue(os.path.isdir(test_dir))
36+
self.assertTrue(os.path.isdir(test_dir))
3437

38+
mock_reg.assert_called_once_with(shutil.rmtree, test_dir, ignore_errors=True)
3539
finally:
3640
if data_dir is not None:
3741
os.environ[MONAI_DATA_DIRECTORY] = data_dir
@@ -51,21 +55,33 @@ def test_data_dir(self):
5155
def test_given_dir(self):
5256
"""Test giving a directory to the function, ensuring it creates the directory."""
5357
with tempfile.TemporaryDirectory() as temp_dir:
54-
selected_dir = f"{temp_dir}/test_inner_dir"
58+
selected_dir = f"{temp_dir}{os.path.sep}test_inner_dir"
59+
5560
test_dir = create_temp_dir(selected_dir)
5661

5762
self.assertTrue(os.path.isdir(selected_dir))
5863
self.assertEqual(test_dir, selected_dir)
64+
self.assertEqual(test_dir, selected_dir)
65+
66+
def test_given_dir_path(self):
67+
"""Test giving a directory as a Path object to the function, ensuring it creates the directory."""
68+
with tempfile.TemporaryDirectory() as temp_dir:
69+
selected_dir = f"{temp_dir}{os.path.sep}test_inner_dir"
70+
71+
test_dir = create_temp_dir(Path(selected_dir))
72+
73+
self.assertTrue(os.path.isdir(selected_dir))
74+
self.assertEqual(test_dir, selected_dir)
5975

6076
def test_finalisation(self):
61-
"""Test the temporary directory is deleted by finalisation using a subprocess."""
77+
"""Test the temporary directory is deleted by finalisation."""
6278
self.finaliser = None
6379

6480
def _register(func, /, *args, **kwargs):
6581
self.finaliser = (func, args, kwargs)
6682

6783
with patch("atexit.register", new=_register), tempfile.TemporaryDirectory() as temp_dir:
68-
selected_dir = f"{temp_dir}/test_inner_dir"
84+
selected_dir = f"{temp_dir}{os.path.sep}test_inner_dir"
6985
test_dir = create_temp_dir(selected_dir, True)
7086

7187
self.assertTrue(os.path.isdir(selected_dir))

0 commit comments

Comments
 (0)