Repository: mhe/pynrrd

Issue:
Print relative data file name in the header
In the detached header, the data file name is printed with abspath. That means, if someone moves the data around, than data file location is invalid. Instead, how about we write the data file with basename?

https://github.com/mhe/pynrrd/blob/master/nrrd/writer.py#L186 and https://github.com/mhe/pynrrd/blob/master/nrrd/writer.py#L191

I think the specification also suggests that:

> Breaking the dataset into a header and one or more data files raises a new concerns, namely that the header file can't know if the data file has been erased, renamed, or moved. NRRD provides no means to overcome these problems once they've been created. On the other hand, moving the header and data files together to a new place is a common operation, and is supported by the special semantics associated with the data filename:
    If the filename (given either directly from "filename", or generated from "format", or listed in a "LIST") does not begins with "/", it is taken to be the filename of the data file, relative to the location of the detached header file. The NRRD reader is responsible for constructing the full data filename from the filename of the detached header and "filename". Obviously, if the detached header has been passed to the reader not as a file name, but as a FILE* (such as stdin), this filename construction is impossible.
    Otherwise, if "filename" does start with "/", then filename gives the full path of the data file. It is passed directly to fopen() without any interpretation.
Note: as of NRRD0004, the signifier of a header-relative file changed from the presence (at the beginning of the filename) of "./", to the absence of "/". Since essentially all uses of detached headers assumed header-relative data files, the explicit "./" flag was deemed unnecessary. With this change, it becomes impossible for a header to refer to the data file relative to the current working directory of the reader (which may be different that the directory of the header), but that's probably a good thing. 


Suggestion:
> header['data file']= os.path.basename(data_filename)

Happy to submit a PR.

cc: @ihnorton 


Discussion:
addisonElliott: We might want to add some tests to fully verify that the new parameter is working as expected. 
codecov-io: # [Codecov](https://codecov.io/gh/mhe/pynrrd/pull/79?src=pr&el=h1) Report
> Merging [#79](https://codecov.io/gh/mhe/pynrrd/pull/79?src=pr&el=desc) into [master](https://codecov.io/gh/mhe/pynrrd/commit/c1c42d97f74ff2230b50df019d033a010f797918?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

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

```diff
@@           Coverage Diff           @@
##           master      #79   +/-   ##
=======================================
  Coverage   88.12%   88.12%           
=======================================
  Files           6        6           
  Lines         362      362           
  Branches      117      117           
=======================================
  Hits          319      319           
  Misses         21       21           
  Partials       22       22
```


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

------

[Continue to review full report at Codecov](https://codecov.io/gh/mhe/pynrrd/pull/79?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/79?src=pr&el=footer). Last update [c1c42d9...106d2f7](https://codecov.io/gh/mhe/pynrrd/pull/79?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tashrifbillah: Added tests, two for absolute path, two for relative path. Basically, I modified the existing detached header tests in a clever way to encompass new tests.

cc: @ihnorton 

Patch:
diff --git a/nrrd/writer.py b/nrrd/writer.py
index d8c0b1c..0a63f59 100644
--- a/nrrd/writer.py
+++ b/nrrd/writer.py
@@ -93,7 +93,7 @@ def _format_field_value(value, field_type):
         raise NRRDError('Invalid field type given: %s' % field_type)
 
 
-def write(filename, data, header={}, detached_header=False, custom_field_map=None,
+def write(filename, data, header={}, detached_header=False, relative_data_path=True, custom_field_map=None,
                           compression_level = 9):
     """Write :class:`numpy.ndarray` to NRRD file
 
@@ -121,6 +121,9 @@ def write(filename, data, header={}, detached_header=False, custom_field_map=Non
         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`
+    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`
     custom_field_map : :class:`dict` (:class:`str`, :class:`str`), optional
         Dictionary used for parsing custom field types where the key is the custom field name and the value is a
         string identifying datatype for the custom field.
@@ -183,12 +186,14 @@ def write(filename, data, header={}, detached_header=False, custom_field_map=Non
             else:
                 raise NRRDError('Invalid encoding specification while writing NRRD file: %s' % header['encoding'])
 
-            header['data file'] = data_filename
+            header['data file'] = os.path.basename(data_filename) \
+                if relative_data_path else os.path.abspath(data_filename)
         else:
             data_filename = header['data file']
     elif filename.endswith('.nrrd') and detached_header:
         data_filename = filename
-        header['data file'] = data_filename
+        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


Test patch:
diff --git a/nrrd/tests/test_writing.py b/nrrd/tests/test_writing.py
index 2b8274e..db1b507 100644
--- a/nrrd/tests/test_writing.py
+++ b/nrrd/tests/test_writing.py
@@ -158,7 +158,8 @@ class TestWritingFunctions(unittest.TestCase):
         output_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.nhdr')
         output_data_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.nrrd')
 
-        nrrd.write(output_data_filename, self.data_input, {u'encoding': 'raw'}, detached_header=True)
+        nrrd.write(output_data_filename, self.data_input, {u'encoding': 'raw'}, detached_header=True,
+                   relative_data_path=False)
 
         # Read back the same file
         data, header = nrrd.read(output_filename)
@@ -198,7 +199,8 @@ class TestWritingFunctions(unittest.TestCase):
         output_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.nhdr')
         output_data_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.raw.gz')
 
-        nrrd.write(output_filename, self.data_input, {u'encoding': 'gz'}, detached_header=False)
+        nrrd.write(output_filename, self.data_input, {u'encoding': 'gz'}, detached_header=False,
+                   relative_data_path=False)
 
         # Read back the same file
         data, header = nrrd.read(output_filename)
@@ -208,7 +210,6 @@ class TestWritingFunctions(unittest.TestCase):
 
     def test_write_detached_bz2(self):
         output_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.nhdr')
-        output_data_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.raw.bz2')
 
         nrrd.write(output_filename, self.data_input, {u'encoding': 'bz2'}, detached_header=False)
 
@@ -216,11 +217,10 @@ class TestWritingFunctions(unittest.TestCase):
         data, header = nrrd.read(output_filename)
         self.assertEqual(self.expected_data, data.tostring(order='F'))
         self.assertEqual(header['encoding'], 'bz2')
-        self.assertEqual(header['data file'], output_data_filename)
+        self.assertEqual(header['data file'], 'testfile_detached_raw.raw.bz2')
 
     def test_write_detached_ascii(self):
         output_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.nhdr')
-        output_data_filename = os.path.join(self.temp_write_dir, 'testfile_detached_raw.txt')
 
         nrrd.write(output_filename, self.data_input, {u'encoding': 'txt'}, detached_header=False)
 
@@ -228,7 +228,7 @@ class TestWritingFunctions(unittest.TestCase):
         data, header = nrrd.read(output_filename)
         self.assertEqual(self.expected_data, data.tostring(order='F'))
         self.assertEqual(header['encoding'], 'txt')
-        self.assertEqual(header['data file'], output_data_filename)
+        self.assertEqual(header['data file'], 'testfile_detached_raw.txt')
 
 
 if __name__ == '__main__':