Repository: pydicom/deid

Issue:
Runaway temp folder appears to be leaked for every clean operation.
In clean.py, line 54-55 a temp folder is created and never deleted:

 if output_folder is None:
            output_folder = tempfile.mkdtemp()

The user of mkdtemp() is responsible for deleting the temporary directory and its contents when done with it.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9fa0995..0a00e65 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ and **Merged pull requests**. Critical items to know are:
 Referenced versions in headers are tagged on Github, in parentheses are for pypi.
 
 ## [vxx](https://github.com/pydicom/deid/tree/master) (master)
+ - need to clean up temporary directory (mkdtemp), issue #68 (0.1.18)
  - fixing issue #65, save for compressed data (0.1.17)
  - matplotlib must be less than or equal to 2.1.2 for install (0.1.16)
  - fixing bug with clean coordinate flipping rectangle
diff --git a/deid/dicom/pixels/clean.py b/deid/dicom/pixels/clean.py
index b01417e..57e0642 100644
--- a/deid/dicom/pixels/clean.py
+++ b/deid/dicom/pixels/clean.py
@@ -24,10 +24,10 @@ SOFTWARE.
 
 from deid.config import DeidRecipe
 from deid.logger import bot
+from deid.utils import get_temporary_name
 from pydicom import read_file
 import matplotlib
 matplotlib.use('pdf')
-import tempfile
 import os
 import re
 
@@ -52,7 +52,7 @@ class DicomCleaner():
                        force=True):
 
         if output_folder is None:
-            output_folder = tempfile.mkdtemp()
+            output_folder = get_temporary_name(prefix="clean")
 
         if font is None:
             font = self.default_font()
@@ -141,15 +141,25 @@ class DicomCleaner():
 
     def _get_clean_name(self, output_folder, extension='dcm'):
         '''return a full path to an output file, with custom folder and
-           extension
+           extension. If the output folder isn't yet created, make it.
+ 
+           Parameters
+           ==========
+           output_folder: the output folder to create, will be created if doesn't
+           exist.
+           extension: the extension of the file to create a name for, should
+           not start with "."
         '''
         if output_folder is None:
             output_folder = self.output_folder
 
+        if not os.path.exists(output_folder):
+            bot.debug('Creating output folder %s' % output_folder)
+            os.mkdir(output_folder)
+
         basename = re.sub('[.]dicom|[.]dcm', '', os.path.basename(self.dicom_file))
         return "%s/cleaned-%s.%s" %(output_folder, basename, extension)
 
-        
     def save_png(self, output_folder=None, image_type="cleaned", title=None):
         '''save an original or cleaned dicom as png to disk.
            Default image_format is "cleaned" and can be set 
@@ -157,8 +167,8 @@ class DicomCleaner():
            flagged) the cleaned image is just a copy of original
         '''
         from matplotlib import pyplot as plt
-        
-        if hasattr(self,image_type):
+
+        if hasattr(self, image_type):
             png_file = self._get_clean_name(output_folder, 'png')
             plt = self.get_figure(image_type=image_type, title=title)
             plt.savefig(png_file)
diff --git a/deid/utils.py b/deid/utils.py
index 636a454..4af24b1 100644
--- a/deid/utils.py
+++ b/deid/utils.py
@@ -29,7 +29,7 @@ import json
 import os
 import re
 import requests
-import json
+import tempfile
 from deid.logger import bot
 from collections import OrderedDict
 import sys
@@ -40,9 +40,9 @@ if sys.version_info[0] < 3:
     from exceptions import OSError
 
 
-######################################################################################
+################################################################################
 # Local commands and requests
-######################################################################################
+################################################################################
 
 def get_installdir():
     '''get_installdir returns the installation directory of the application
@@ -50,10 +50,30 @@ def get_installdir():
     return os.path.abspath(os.path.dirname(__file__))
 
 
+def get_temporary_name(prefix=None, ext=None):
+    '''get a temporary name, can be used for a directory or file. This does so
+       without creating the file, and adds an optional prefix
+  
+       Parameters
+       ==========
+       prefix: if defined, add the prefix after deid
+       ext: if defined, return the file extension appended. Do not specify "."
+    '''
+    deid_prefix = 'deid-'
+    if prefix:
+        deid_prefix = 'deid-%s-' % prefix
+
+    tmpname = os.path.join(tempfile.gettempdir(), 
+                           '%s%s' % (deid_prefix,
+                                     next(tempfile._get_candidate_names())))
+    if ext:
+        tmpname = '%s.%s' % (tmpname, ext)
+    return tmpname
+
 
-############################################################################
-## FILE OPERATIONS #########################################################
-############################################################################
+################################################################################
+## FILE OPERATIONS #############################################################
+################################################################################
 
 def write_file(filename,content,mode="w"):
     '''write_file will open a file, "filename" and write content, "content"
diff --git a/deid/version.py b/deid/version.py
index da29ba8..e6987c8 100644
--- a/deid/version.py
+++ b/deid/version.py
@@ -1,5 +1,5 @@
 '''
-Copyright (c) 2017 Vanessa Sochat
+Copyright (c) 2017-2018 Vanessa Sochat
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal
@@ -20,7 +20,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
 '''
 
-__version__ = "0.1.17"
+__version__ = "0.1.18"
 AUTHOR = 'Vanessa Sochat'
 AUTHOR_EMAIL = 'vsochat@stanford.edu'
 NAME = 'deid'


Test patch:
diff --git a/deid/tests/test_utils.py b/deid/tests/test_utils.py
index 9d6fd37..6cc5905 100644
--- a/deid/tests/test_utils.py
+++ b/deid/tests/test_utils.py
@@ -28,6 +28,7 @@ SOFTWARE.
 '''
 
 from deid.utils import get_installdir
+ 
 from numpy.testing import (
     assert_array_equal, 
     assert_almost_equal, 
@@ -52,6 +53,19 @@ class TestUtils(unittest.TestCase):
         shutil.rmtree(self.tmpdir)
         print("\n######################END########################")
 
+    def test_get_temporary_name(self):
+        '''test_get_temporary_name will test the generation of a temporary 
+           file name.
+        '''
+        from deid.utils import get_temporary_name
+        print("Testing utils.get_temporary_name...")
+        tmpname = get_temporary_name()
+        self.assertTrue(not os.path.exists(tmpname))
+        self.assertTrue('deid' in tmpname)
+        tmpname = get_temporary_name(prefix='clean')
+        self.assertTrue('deid-clean' in tmpname)
+        tmpname = get_temporary_name(ext='.dcm')
+        self.assertTrue(tmpname.endswith('.dcm'))
 
     def test_write_read_files(self):
         '''test_write_read_files will test the functions