Repository: mhe/pynrrd

Issue:
Modify 'data file' whenever detached_header=True
We should omit the if-else below:
https://github.com/mhe/pynrrd/blob/master/nrrd/writer.py#L190

Omitting the condition should also address the TODO below:
https://github.com/mhe/pynrrd/blob/master/nrrd/writer.py#L210

I experienced this while modifying an NHDR and saving it as another NHDR. In this case, the modified detached data is not saved where my modified NHDR is. 

Correction:
Detached data is saved in the working directory.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=h1) Report
> Merging [#104](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=desc) into [master](https://codecov.io/gh/mhe/pynrrd/commit/6faf838d36094955adafc6c60c8e115dff2f2939?src=pr&el=desc) will **increase** coverage by `0.26%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/mhe/pynrrd/pull/104/graphs/tree.svg?width=650&token=ktfNONePUI&height=150&src=pr)](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=tree)

```diff
@@           Coverage Diff            @@
##           master   #104      +/-   ##
========================================
+ Coverage   99.73%   100%   +0.26%     
========================================
  Files           6      6              
  Lines         383    385       +2     
  Branches      124    125       +1     
========================================
+ Hits          382    385       +3     
+ Partials        1      0       -1
```


| [Impacted Files](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [nrrd/writer.py](https://codecov.io/gh/mhe/pynrrd/pull/104/diff?src=pr&el=tree#diff-bnJyZC93cml0ZXIucHk=) | `100% <100%> (+0.8%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=footer). Last update [6faf838...855aec4](https://codecov.io/gh/mhe/pynrrd/pull/104?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/nrrd/writer.py b/nrrd/writer.py
index 24d78b0..f459451 100644
--- a/nrrd/writer.py
+++ b/nrrd/writer.py
@@ -109,9 +109,9 @@ def write(filename, data, header=None, detached_header=False, relative_data_path
 
     .. note::
             The following fields are automatically generated based on the :obj:`data` parameter ignoring these values
-            in the :obj:`header`: 'type', 'endian', 'dimension', 'sizes'. In addition, the generated fields will be
-            added to the given :obj:`header`. Thus, one can check the generated fields by viewing the passed
-            :obj:`header`.
+            in the :obj:`header`: 'type', 'endian', 'dimension', 'sizes', and 'data file'. In addition, the generated
+            fields will be added to the given :obj:`header`. Thus, one can check the generated fields by viewing the
+            passed :obj:`header`.
 
     .. note::
             The default encoding field used if not specified in :obj:`header` is 'gzip'.
@@ -129,8 +129,10 @@ def write(filename, data, header=None, detached_header=False, relative_data_path
         Filename of the NRRD file
     data : :class:`numpy.ndarray`
         Data to save to the NRRD file
-    detached_header : :obj:`bool`, optional
-        Whether the header and data should be saved in separate files. Defaults to :obj:`False`
+    detached_header : :obj:`bool` or :obj:`str`, optional
+        Whether the header and data should be saved in separate files. Defaults to :obj:`False`. If a :obj:`str` is
+        given this specifies the path to the datafile. This path will ONLY be used if the given filename ends with nhdr
+        (i.e. the file is a header)
     relative_data_path : :class:`bool`
         Whether the data filename in detached header is saved with a relative path or absolute path.
         This parameter is ignored if there is no detached header. Defaults to :obj:`True`
@@ -180,22 +182,24 @@ def write(filename, data, header=None, detached_header=False, relative_data_path
     if 'encoding' not in header:
         header['encoding'] = 'gzip'
 
-    # If 'datafile' is specified, then we rename to 'data file'
-    # The standard seems to advocate for 'data file' OVER 'datafile'
+    # Remove detached data filename from the header
     if 'datafile' in header:
-        header['data file'] = header.pop('datafile')
+        header.pop('datafile')
+
+    if 'data file' in header:
+        header.pop('data file')
 
     # A bit of magic in handling options here.
     # If *.nhdr filename provided, this overrides `detached_header=False`
     # If *.nrrd filename provided AND detached_header=True, separate header and data files written.
-    # If detached_header=True and data file is present, then write the files separately
     # For all other cases, header & data written to same file.
     if filename.endswith('.nhdr'):
-        detached_header = True
-
-        # TODO This will cause issues for relative data files because it will not save in the correct spot
-        data_filename = header.get('datafile', None)
-        if not data_filename:
+        if isinstance(detached_header, str):
+            # Utilize the detached_header if a string was given as the path
+            # Note: An absolute path is obtained and assumed to be relative to the current path of the running Python
+            # program
+            data_filename = os.path.abspath(detached_header)
+        else:
             # Get the base filename without the extension
             base_filename = os.path.splitext(filename)[0]
 
@@ -212,13 +216,17 @@ def write(filename, data, header=None, detached_header=False, relative_data_path
             else:
                 raise NRRDError('Invalid encoding specification while writing NRRD file: %s' % header['encoding'])
 
-            header['data file'] = os.path.basename(data_filename) \
-                if relative_data_path else os.path.abspath(data_filename)
+        # Update the data file field in the header with the path of the detached data
+        # TODO This will cause problems when the user specifies a relative data path and gives a custom path OUTSIDE
+        #  of the current directory.
+        header['data file'] = os.path.basename(data_filename) \
+            if relative_data_path else os.path.abspath(data_filename)
+        detached_header = True
     elif filename.endswith('.nrrd') and detached_header:
         data_filename = filename
+        filename = '%s.nhdr' % os.path.splitext(filename)[0]
         header['data file'] = os.path.basename(data_filename) \
             if relative_data_path else os.path.abspath(data_filename)
-        filename = '%s.nhdr' % os.path.splitext(filename)[0]
     else:
         # Write header & data as one file
         data_filename = filename


Test patch:
diff --git a/nrrd/tests/test_writing.py b/nrrd/tests/test_writing.py
index 7c0c1c7..7da1019 100644
--- a/nrrd/tests/test_writing.py
+++ b/nrrd/tests/test_writing.py
@@ -313,7 +313,7 @@ class TestWritingFunctions(object):
     def test_write_detached_datafile_check(self):
         output_filename = os.path.join(self.temp_write_dir, 'testfile_detached.nhdr')
 
-        nrrd.write(output_filename, self.data_input, {'datafile': 'testfile_detached.gz'}, detached_header=True,
+        nrrd.write(output_filename, self.data_input, {'datafile': 'testfile_detachedWRONG.gz'}, detached_header=True,
                    index_order=self.index_order)
 
         # Read back the same file
@@ -323,13 +323,36 @@ class TestWritingFunctions(object):
     def test_write_detached_datafile_check2(self):
         output_filename = os.path.join(self.temp_write_dir, 'testfile_detached.nhdr')
 
-        nrrd.write(output_filename, self.data_input, {'data file': 'testfile_detached.gz'}, detached_header=True,
+        nrrd.write(output_filename, self.data_input, {'data file': 'testfile_detachedWRONG.gz'}, detached_header=True,
                    index_order=self.index_order)
 
         # Read back the same file
         data, header = nrrd.read(output_filename, index_order=self.index_order)
         self.assertEqual(header['data file'], 'testfile_detached.raw.gz')
 
+    def test_write_detached_datafile_custom_name(self):
+        output_filename = os.path.join(self.temp_write_dir, 'testfile_detached.nhdr')
+        # Specify a custom path to write the
+        output_header_filename = os.path.join(self.temp_write_dir, 'testfile_detachedDifferent.gz')
+
+        nrrd.write(output_filename, self.data_input, detached_header=output_header_filename,
+                   index_order=self.index_order)
+
+        # Read back the same file
+        data, header = nrrd.read(output_filename, index_order=self.index_order)
+        self.assertEqual(header['data file'], 'testfile_detachedDifferent.gz')
+
+    def test_write_check_remove_datafile(self):
+        output_filename = os.path.join(self.temp_write_dir, 'testfile.nrrd')
+
+        nrrd.write(output_filename, self.data_input, {'data file': 'testfile_detached.gz'}, detached_header=False,
+                   index_order=self.index_order)
+
+        # Read back the same file
+        # The 'data file' parameter should be missing since this is NOT a detached file
+        data, header = nrrd.read(output_filename, index_order=self.index_order)
+        self.assertFalse('data file' in header)
+
 
 class TestWritingFunctionsFortran(TestWritingFunctions, unittest.TestCase):
     index_order = 'F'