From ffb1632c56307d88755c87d7f4ff303e842fe8a5 Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Thu, 7 Jul 2016 19:02:41 -0500
Subject: [PATCH 1/8] Close file in h5py if the file mode changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This ensures that the file is reopened with the correct mode (‘r’ or
‘rw’) during the next file operation.
---
 src/nexusformat/nexus/tree.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 4e4d322..75fe26a 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -396,6 +396,12 @@ def close(self):
         if self._file.id:
             self._file.close()
 
+    def isopen(self):
+        if self._file.id:
+            return True
+        else:
+            return False
+
     def readfile(self):
         """
         Reads the NeXus file structure from the file and returns a tree of 
@@ -746,8 +752,12 @@ def mode(self):
     def mode(self, mode):
         if mode == 'rw' or mode == 'r+':
             self._mode = 'rw'
+            if self._file.id and self._file.mode == 'r':
+                self.close()
         else:
             self._mode = 'r'   
+            if self._file.id and self._file.mode == 'r+':
+                self.close()
 
     @property
     def attrs(self):

From 0c31e898e3d27559b9fe4b6510e00cbc008ec92e Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Thu, 7 Jul 2016 20:54:53 -0500
Subject: [PATCH 2/8] Keep HDF5 file closed as much as possible
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This extends the use of ‘with’ statements to access information from
the HDF5 file. By default, the NXobject nxfile property does not reopen
the file, unless there is an attempt to access data. This is now
handled by the NXFile ‘file’ property.
---
 src/nexusformat/nexus/tree.py | 63 ++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 75fe26a..54db7a7 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -344,10 +344,12 @@ def __init__(self, name, mode=None, **kwds):
             self._file = h5.File(name, mode, **kwds)
             self._mode = 'rw'
         else:
-            if mode == 'rw':
+            if mode == 'rw' or mode == 'r+':
+                self._mode = 'rw'
                 mode = 'r+'
+            else:
+                self._mode = 'r'
             self._file = h5.File(name, mode, **kwds)
-            self.mode = mode   
         self._filename = self._file.filename                             
         self._path = ''
 
@@ -357,19 +359,19 @@ def __repr__(self):
 
     def __getitem__(self, key):
         """Returns an object from the NeXus file."""
-        return self._file.get(key)
+        return self.file.get(key)
 
     def __setitem__(self, key, value):
         """Sets an object value in the NeXus file."""
-        self._file[key] = value
+        self.file[key] = value
 
     def __delitem__(self, name):
         """ Delete an item from a group. """
-        del self._file[name]
+        del self.file[name]
 
     def __contains__(self, key):
         """Implements 'k in d' test"""
-        return self._file.__contains__(key)
+        return self.file.__contains__(key)
 
     def __enter__(self):
         return self.open()
@@ -378,10 +380,10 @@ def __exit__(self, *args):
         self.close()
 
     def get(self, *args, **kwds):
-        return self._file.get(*args, **kwds)
+        return self.file.get(*args, **kwds)
 
     def copy(self, *args, **kwds):
-        self._file.copy(*args, **kwds)
+        self.file.copy(*args, **kwds)
 
     def open(self, **kwds):
         if not self._file.id:
@@ -601,7 +603,7 @@ def _writedata(self, data):
                 with _file as f:
                     f.copy(_path, self[self.nxparent], self.nxpath)
             else:
-                self._file.copy(_path, self[self.nxparent], self.nxpath)
+                self.file.copy(_path, self[self.nxparent], self.nxpath)
             data._uncopied_data = None
         elif data._memfile:
             data._memfile.copy('data', self[self.nxparent], self.nxpath)
@@ -693,12 +695,12 @@ def copyfile(self, input_file):
 
     def _rootattrs(self):
         from datetime import datetime
-        self._file.attrs['file_name'] = self.filename
-        self._file.attrs['file_time'] = datetime.now().isoformat()
-        self._file.attrs['HDF5_Version'] = h5.version.hdf5_version
-        self._file.attrs['h5py_version'] = h5.version.version
+        self.file.attrs['file_name'] = self.filename
+        self.file.attrs['file_time'] = datetime.now().isoformat()
+        self.file.attrs['HDF5_Version'] = h5.version.hdf5_version
+        self.file.attrs['h5py_version'] = h5.version.version
         from .. import __version__
-        self._file.attrs['nexusformat_version'] = __version__
+        self.file.attrs['nexusformat_version'] = __version__
 
     def update(self, item):
         self.nxpath = item.nxpath
@@ -719,7 +721,7 @@ def update(self, item):
             self.nxpath = item.nxpath
 
     def rename(self, old_path, new_path):
-        self._file['/'].move(old_path, new_path)
+        self.file['/'].move(old_path, new_path)
 
     def _islink(self):
         _target, _ = self._readlink()
@@ -738,10 +740,12 @@ def _isexternal(self):
     @property
     def filename(self):
         """File name on disk"""
-        return self._file.filename
+        return self.file.filename
 
     @property
     def file(self):
+        if not self._file.id:
+            self.open()
         return self._file
 
     @property
@@ -752,11 +756,11 @@ def mode(self):
     def mode(self, mode):
         if mode == 'rw' or mode == 'r+':
             self._mode = 'rw'
-            if self._file.id and self._file.mode == 'r':
+            if self.file.id and self.file.mode == 'r':
                 self.close()
         else:
             self._mode = 'r'   
-            if self._file.id and self._file.mode == 'r+':
+            if self.file.id and self.file.mode == 'r+':
                 self.close()
 
     @property
@@ -1432,10 +1436,10 @@ def nxentry(self):
     @property
     def nxfile(self):
         if self._file:
-            return self._file.open()
+            return self._file
         _root = self.nxroot
         if _root._file:
-            return _root._file.open()
+            return _root._file
         elif _root._filename:
             return NXFile(_root._filename, _root._mode)
         else:
@@ -1444,7 +1448,8 @@ def nxfile(self):
     @property
     def nxfilename(self):
         try:
-            return self.nxfile[self.nxpath].file.filename
+            with self.nxfile as f:
+                return f[self.nxpath].file.filename
         except Exception:
             return ''
 
@@ -2558,7 +2563,8 @@ def shape(self, value):
     @property
     def compression(self):
         if self.nxfilemode:
-            self._compression = self.nxfile[self.nxpath].compression
+            with self.nxfile as f:
+                self._compression = f[self.nxpath].compression
         elif self._memfile:
             self._compression = self._memfile['data'].compression
         return self._compression
@@ -2574,7 +2580,8 @@ def compression(self, value):
     @property
     def fillvalue(self):
         if self.nxfilemode:
-            self._fillvalue = self.nxfile[self.nxpath].fillvalue
+            with self.nxfile as f:
+                self._fillvalue = f[self.nxpath].fillvalue
         elif self._memfile:
             self._fillvalue = self._memfile['data'].fillvalue
         return self._fillvalue
@@ -2590,7 +2597,8 @@ def fillvalue(self, value):
     @property
     def chunks(self):
         if self.nxfilemode:
-            self._chunks = self.nxfile[self.nxpath].chunks
+            with self.nxfile as f:
+                self._chunks = f[self.nxpath].chunks
         elif self._memfile:
             self._chunks = self._memfile['data'].chunks
         return self._chunks
@@ -2608,7 +2616,8 @@ def chunks(self, value):
     @property
     def maxshape(self):
         if self.nxfilemode:
-            self._maxshape = self.nxfile[self.nxpath].maxshape
+            with self.nxfile as f:
+                self._maxshape = f[self.nxpath].maxshape
         elif self._memfile:
             self._maxshape = self._memfile['data'].maxshape
         return self._maxshape
@@ -3743,11 +3752,13 @@ def lock(self):
         """Make the tree readonly"""
         if self._filename:
             self._mode = self._file.mode = 'r'
+            self.set_changed()
 
     def unlock(self):
         """Make the tree modifiable"""
         if self._filename:
             self._mode = self._file.mode = 'rw'
+            self.set_changed()
 
     def backup(self, filename=None, dir=None):
         """Backup the NeXus file.
@@ -3813,7 +3824,7 @@ def plottable_data(self):
     @property
     def nxfile(self):
         if self._file:
-            return self._file.open()
+            return self._file
         elif self._filename:
             return NXFile(self._filename, self._mode)
         else:

From d01ddc5a788410592f12aa63ac2765965d2329a3 Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Tue, 19 Jul 2016 15:51:30 -0500
Subject: [PATCH 3/8] Miscellaneous improvements

1) Stores the filename as an absolute path in the NXFile object.
2) Allows NXroot objects to be saved in place. Before, the saved NXroot
object was returned from the function, but the original tree was not
updated.
3) Returns naturally sorted names when listing group items of a
particular class, e.g., the list returned by some_group.NXentry sorts
by the final number so that 2 comes before 10.
---
 src/nexusformat/nexus/tree.py | 38 +++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 54db7a7..892afa3 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -297,6 +297,11 @@ def is_text(value):
         return False
 
 
+def natural_sort(key):
+    """Sort numbers according to their value, not their first character"""
+    return [int(t) if t.isdigit() else t for t in re.split(r'(\d+)', key)]    
+
+
 class NeXusError(Exception):
     """NeXus Error"""
     pass
@@ -336,6 +341,7 @@ def __init__(self, name, mode=None, **kwds):
         """
         Creates an h5py File object for reading and writing.
         """
+        name = os.path.abspath(name)
         if mode == 'w4' or mode == 'wx':
             raise NeXusError('Only HDF5 files supported')
         elif mode == 'w' or mode == 'w-' or mode == 'w5':
@@ -351,7 +357,7 @@ def __init__(self, name, mode=None, **kwds):
                 self._mode = 'r'
             self._file = h5.File(name, mode, **kwds)
         self._filename = self._file.filename                             
-        self._path = ''
+        self._path = '/'
 
     def __repr__(self):
         return '<NXFile "%s" (mode %s)>' % (os.path.basename(self._filename),
@@ -1326,7 +1332,16 @@ def save(self, filename=None, mode='w'):
             nx_file.writefile(root)
             root = nx_file.readfile()
             nx_file.close()
-            return root
+            if self.nxclass == "NXroot":
+                self._file = nx_file
+                self._entries = root.entries
+                self._attrs = root.attrs
+                self._filename = self._file._filename
+                self._mode = self._file._mode
+                return self
+            else:
+                return root
+            self.set_changed()
         else:
             raise NeXusError("No output file specified")
 
@@ -1398,10 +1413,10 @@ def nxgroup(self):
         return self._group
 
     def _getpath(self):
-        if self.nxgroup is None:
-            return ""
-        elif self.nxclass == 'NXroot':
+        if self.nxclass == 'NXroot':
             return "/"
+        elif self.nxgroup is None:
+            return ""
         elif isinstance(self.nxgroup, NXroot):
             return "/" + self.nxname
         else:
@@ -3253,6 +3268,13 @@ def has_key(self, name):
         """
         return self.entries.has_key(name)
 
+    def component(self, nxclass):
+        """
+        Finds all child objects that have a particular class.
+        """
+        return [self.entries[i] for i in sorted(self.entries, key=natural_sort)
+                if self.entries[i].nxclass==nxclass]
+
     def insert(self, value, name='unknown'):
         """
         Adds an attribute to the group.
@@ -3440,12 +3462,6 @@ def implot(self, **opts):
         except AttributeError:
             raise NeXusError("Data cannot be plotted")
 
-    def component(self, nxclass):
-        """
-        Finds all child objects that have a particular class.
-        """
-        return [E for _name,E in self.items() if E.nxclass==nxclass]
-
     def signals(self):
         """
         Returns a dictionary of NXfield's containing signal data.

From f1531ac1b9fac5b6d1fa315dd73550a311a6a222 Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Wed, 20 Jul 2016 17:52:17 -0500
Subject: [PATCH 4/8] Minor tweaks

The most important is for nxfilename to return the first value of
_filename in a recursive search through parent groups. This helps to
ensure that external links return the correct filename. This also
reduces the number of times the HDF5 file has to be opened and closed.
---
 src/nexusformat/nexus/tree.py | 41 +++++++++++++++--------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 892afa3..7db568b 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -423,7 +423,7 @@ def readfile(self):
         root = self._readgroup('root')
         root._group = None
         root._file = self
-        root._filename = self.filename
+        root._filename = self._filename
         root._mode = self._mode = _mode
         return root
 
@@ -1335,6 +1335,8 @@ def save(self, filename=None, mode='w'):
             if self.nxclass == "NXroot":
                 self._file = nx_file
                 self._entries = root.entries
+                for entry in self._entries:
+                    self._entries[entry]._group = self
                 self._attrs = root.attrs
                 self._filename = self._file._filename
                 self._mode = self._file._mode
@@ -1412,7 +1414,8 @@ def nxname(self, value):
     def nxgroup(self):
         return self._group
 
-    def _getpath(self):
+    @property
+    def nxpath(self):
         if self.nxclass == 'NXroot':
             return "/"
         elif self.nxgroup is None:
@@ -1426,10 +1429,6 @@ def _getpath(self):
             else:
                 return self.nxname
 
-    @property
-    def nxpath(self):
-        return self._getpath()
-
     @property
     def nxroot(self):
         if self._group is None or isinstance(self, NXroot):
@@ -1462,11 +1461,12 @@ def nxfile(self):
 
     @property
     def nxfilename(self):
-        try:
-            with self.nxfile as f:
-                return f[self.nxpath].file.filename
-        except Exception:
-            return ''
+        if self._filename is not None:
+            return os.path.abspath(self._filename)
+        elif self._group is not None:
+            return self._group.nxfilename
+        else:
+            return None
 
     @property
     def nxfilemode(self):
@@ -2655,14 +2655,8 @@ def size(self):
 
     @property
     def safe_attrs(self):
-        _attrs = copy(self.attrs)
-        if 'target' in _attrs:
-            del _attrs['target']
-        if 'signal' in _attrs:
-            del _attrs['signal']
-        if 'axes' in _attrs:
-            del _attrs['axes']
-        return _attrs
+        return {key: self.attrs[key] for key in self.attrs 
+                if (key != 'target' and key != 'signal' and key != 'axes')}
 
     @property
     def reversed(self):
@@ -3547,8 +3541,6 @@ def __init__(self, target=None, file=None, name=None, group=None):
             if name is None:
                 self._name = target.nxname
             self._target = target.nxpath
-            if file is None:
-                self._filename = target.nxfilename
             if isinstance(target, NXfield):
                 self.__class__ = NXlinkfield
             elif isinstance(target, NXgroup):
@@ -3594,9 +3586,10 @@ def _str_tree(self, indent=0, attrs=False, recursive=False):
         return self._str_name(indent=indent)
 
     def update(self):
-        if (self._filename and os.path.exists(self._filename) and
-            self.nxroot.nxfilemode == 'rw'):
-            with NXFile(self.nxroot.nxfilename) as f:
+        root = self.nxroot
+        filename, mode = root.nxfilename, root.nxfilemode
+        if (filename is not None and os.path.exists(filename) and mode == 'rw'):
+            with NXFile(filename) as f:
                 f.update(self)
                 item = f.readitem()
                 if isinstance(item, NXfield):

From 9adcc94fc6a244a87c4266898a85303e781b269e Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Thu, 21 Jul 2016 14:18:31 -0500
Subject: [PATCH 5/8] Return correct absolute file path for external links

This uses the path of self._filename relative to the root filename.
This was affected by the previous definition of nxfilename.
---
 src/nexusformat/nexus/tree.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 7db568b..2b2cff0 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -3618,6 +3618,19 @@ def nxlink(self):
         except Exception:
             return self
 
+    @property
+    def nxfilename(self):
+        if self._filename is not None:
+            if os.path.isabs(self._filename):
+                return self._filename
+            else:
+                return os.path.abspath(os.path.join(
+                    os.path.dirname(self.nxroot.nxfilename), self._filename))
+        elif self._group is not None:
+            return self._group.nxfilename
+        else:
+            return None
+
     @property
     def nxfilemode(self):
         if self._filename is not None:

From 2dc0d6e16139b2f2056a8dbc7153e24967a32903 Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Fri, 22 Jul 2016 12:59:11 -0500
Subject: [PATCH 6/8] Fixes saves
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Saving files has been made more robust by ensuring that the HDF5 file
is closed after each writefile and readfile operation. This also
ensures that internal links have the ‘target’ attribute added to the
parent NXfield and that attributes are consistently updated.
---
 src/nexusformat/nexus/tree.py | 81 ++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 2b2cff0..1dd9236 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -614,7 +614,7 @@ def _writedata(self, data):
         elif data._memfile:
             data._memfile.copy('data', self[self.nxparent], self.nxpath)
             data._memfile = None
-        elif data.nxfilemode and data.nxfile.filename != self.filename:
+        elif data.nxfile and data.nxfile.filename != self.filename:
             data.nxfile.copy(data.nxpath, self[self.nxparent])
         elif data.dtype is not None:
             if data.nxname not in self[self.nxparent]:
@@ -654,6 +654,8 @@ def _writelinks(self, links):
         # link sources to targets
         for path, target in links:
             if path != target and path not in self['/'] and target in self['/']:
+                if 'target' not in self[target].attrs:
+                    self[target].attrs['target'] = target
                 self[path] = self[target]
 
     def readitem(self):
@@ -948,7 +950,7 @@ class AttrDict(dict):
 
     def __init__(self, parent=None, attrs={}):
         super(AttrDict, self).__init__()
-        self.parent = parent
+        self._parent = parent
         self._setattrs(attrs)
 
     def _setattrs(self, attrs):
@@ -962,26 +964,26 @@ def __setitem__(self, key, value):
         if value is None:
             return
         if isinstance(value, NXattr):
-            super(AttrDict, self).__setitem__(key, value)
+            super(AttrDict, self).__setitem__(text(key), value)
         else:
-            super(AttrDict, self).__setitem__(key, NXattr(value))
-        try:
-            if self.parent.nxfilemode == 'rw':
-                with self.parent.nxfile as f:
-                    f.update(self, self.parent.nxpath)
-        except Exception:
-            pass
+            super(AttrDict, self).__setitem__(text(key), NXattr(value))
+        if self._parent.nxfilemode == 'rw':
+            with self._parent.nxfile as f:
+                f.update(self)
 
     def __delitem__(self, key):
         super(AttrDict, self).__delitem__(key)
         try:
-            if self.parent.nxfilemode == 'rw':
-                with self.parent.nxfile as f:
-                    f.nxpath = self.parent.nxpath
+            if self._parent.nxfilemode == 'rw':
+                with self._parent.nxfile as f:
+                    f.nxpath = self._parent.nxpath
                     del f[f.nxpath].attrs[key]
         except Exception:
             pass
 
+    @property
+    def nxpath(self):
+        return self._parent.nxpath
 
 class NXattr(object):
 
@@ -1280,7 +1282,7 @@ def rename(self, name):
             with self.nxfile as f:
                 f.rename(path, self.nxpath)
 
-    def save(self, filename=None, mode='w'):
+    def save(self, filename=None, mode='w-'):
         """
         Saves the NeXus object to a data file.
         
@@ -1327,23 +1329,20 @@ def save(self, filename=None, mode='w'):
             elif self.nxclass == "NXentry":
                 root = NXroot(self)
             else:
-                root = NXroot(NXentry(self))
-            nx_file = NXFile(filename, mode)
-            nx_file.writefile(root)
-            root = nx_file.readfile()
-            nx_file.close()
-            if self.nxclass == "NXroot":
-                self._file = nx_file
-                self._entries = root.entries
-                for entry in self._entries:
-                    self._entries[entry]._group = self
-                self._attrs = root.attrs
-                self._filename = self._file._filename
-                self._mode = self._file._mode
-                return self
+                root = NXroot(NXentry(self)) 
+            if mode != 'w':
+                write_mode = 'w-'
+            else:
+                write_mode = 'w'
+            with NXFile(filename, write_mode) as f:
+                f.writefile(root)
+            if mode == 'w' or mode == 'w-':
+                root._mode = 'rw'
             else:
-                return root
+                root._mode = mode
+            root.nxfile = filename
             self.set_changed()
+            return root
         else:
             raise NeXusError("No output file specified")
 
@@ -3536,7 +3535,9 @@ def __init__(self, target=None, file=None, name=None, group=None):
         self._attrs = AttrDict(self)
         self._entries = {}
         if isinstance(target, NXobject):
-            if isinstance(target, NXlink):
+            if file is not None:
+                raise NeXusError("For external links, specify the target as a path")
+            elif isinstance(target, NXlink):
                 raise NeXusError("Cannot link to another NXlink object")
             if name is None:
                 self._name = target.nxname
@@ -3853,16 +3854,19 @@ def nxfile(self):
             return None
 
     @nxfile.setter
-    def nxfile(self, value):
-        if os.path.exists(value):
-            self._filename = value
-            self._file = NXFile(value, self._mode)
-            root = self._file.readfile()
-            self._entries = root.entries
-            self._attrs = root.attrs
+    def nxfile(self, filename):
+        if os.path.exists(filename):
+            self._filename = os.path.abspath(filename)
+            with NXFile(self._filename, 'r') as f:
+                root = f.readfile()
+            self._entries = root._entries
+            for entry in self._entries:
+                self._entries[entry]._group = self
+            self._attrs._setattrs(root.attrs)
+            self._file = NXFile(self._filename, self._mode)
             self.set_changed()
         else:
-            raise NeXusError("'%s' does not exist")
+            raise NeXusError("'%s' does not exist" % os.path.abspath(filename))
 
     @property
     def nxbackup(self):
@@ -4737,7 +4741,6 @@ def save(filename, group, mode='w'):
     else:
         tree = NXroot(NXentry(group))
     with NXFile(filename, mode) as f:
-        f = NXFile(filename, mode)
         f.writefile(tree)
         f.close()
  

From 69fdccf4315f340ae07d7ad6b226ee8e9aa6699b Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Sun, 24 Jul 2016 20:28:06 -0500
Subject: [PATCH 7/8] Fix handling of internal links within external links

If there is an internal link in an HDF5 file that is externally linked,
it could not be resolved because the NXFile _readlink function did not
return an external filename. This also makes sure that the NXgroup
insert function copies the values from the linked field if it is an
external link.
---
 src/nexusformat/nexus/tree.py | 40 ++++++++++++++---------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 1dd9236..2116f8e 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -451,12 +451,18 @@ def _readnxclass(self, attrs):
 
     def _readlink(self):
         if self._isexternal():
-            link = self.get(self.nxpath, getlink=True)
-            return link.path, link.filename
-        elif 'target' in self.attrs and self.attrs['target'] != self.nxpath:
-            return self.attrs['target'], None
+            _link = self.get(self.nxpath, getlink=True)
+            _target, _filename = _link.path, _link.filename
+        elif 'target' in self.attrs:
+            _target = self.attrs['target']
+            _filename = self.get(self.nxpath).file.filename
+            if _filename == self.filename:
+                _filename = None
+                if  _target == self.nxpath:
+                    _target = None
         else:
-            return None, None
+            _target, _filename = None, None
+        return _target, _filename
 
     def _readchildren(self):
         children = {}
@@ -483,8 +489,8 @@ def _readgroup(self, name):
         else:
             nxclass = 'NXgroup'
         children = self._readchildren()
-        if self.nxpath != '/' and self._islink():
-            _target, _filename = self._readlink()
+        _target, _filename = self._readlink()
+        if self.nxpath != '/' and _target is not None:
             group = NXlinkgroup(nxclass=nxclass, name=name, attrs=attrs,
                                 entries=children, 
                                 target=_target, file=_filename)
@@ -502,8 +508,8 @@ def _readdata(self, name):
         """
         # Finally some data, but don't read it if it is big
         # Instead record the location, type and size
-        if self._islink():
-            _target, _filename = self._readlink()
+        _target, _filename = self._readlink()
+        if _target is not None:
             if _filename is not None:
                 try:
                     value, shape, dtype, attrs = self.readvalues()
@@ -731,13 +737,6 @@ def update(self, item):
     def rename(self, old_path, new_path):
         self.file['/'].move(old_path, new_path)
 
-    def _islink(self):
-        _target, _ = self._readlink()
-        if _target is not None:
-            return True
-        else:
-            return False
-
     def _isexternal(self):
         try:
             return (self.get(self.nxpath, getclass=True, getlink=True)
@@ -3632,13 +3631,6 @@ def nxfilename(self):
         else:
             return None
 
-    @property
-    def nxfilemode(self):
-        if self._filename is not None:
-            return 'r'
-        else:
-            return self.nxlink.nxfilemode
-
 
 class NXlinkfield(NXlink, NXfield):
 
@@ -3672,7 +3664,7 @@ def _str_tree(self, indent=0, attrs=False, recursive=False):
     @property
     def nxlink(self):
         try:
-            if self._filename is not None:
+            if self.nxfilename != self.nxroot.nxfilename:
                 return self
             else:
                return self.nxroot[self._target]

From 83730018bbc936bdcff684d6acdb6401c5f538b6 Mon Sep 17 00:00:00 2001
From: Ray Osborn <rayosborn@mac.com>
Date: Wed, 27 Jul 2016 14:58:55 -0500
Subject: [PATCH 8/8] Fix the insertion of links in external files

If the object to be inserted is a link in an external file, the parent
of the link is copied.
---
 src/nexusformat/nexus/tree.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/nexusformat/nexus/tree.py b/src/nexusformat/nexus/tree.py
index 2116f8e..d8b8cc4 100644
--- a/src/nexusformat/nexus/tree.py
+++ b/src/nexusformat/nexus/tree.py
@@ -3271,15 +3271,20 @@ def insert(self, value, name='unknown'):
         """
         Adds an attribute to the group.
 
-        If it is not a valid NeXus object (NXfield or NXgroup), the attribute
-        is converted to an NXfield.
+        If it is not a valid NeXus object, the attribute is converted to an 
+        NXfield. If the object is an internal link within an externally linked
+        file, the linked object in the external file is copied.
         """
         if isinstance(value, NXobject):
             if name == 'unknown': 
                 name = value.nxname
             if name in self.entries:
                 raise NeXusError("'%s' already exists in group" % name)
-            self[name] = value
+            if (isinstance(value, NXlink) and 
+                value.nxfilename != self.nxfilename):
+                self[name] = value.nxlink
+            else:
+                self[name] = value
         else:
             if name in self.entries:
                 raise NeXusError("'%s' already exists in group" % name)
@@ -3535,7 +3540,8 @@ def __init__(self, target=None, file=None, name=None, group=None):
         self._entries = {}
         if isinstance(target, NXobject):
             if file is not None:
-                raise NeXusError("For external links, specify the target as a path")
+                raise NeXusError(
+                    "Use the NXgroup makelink function for external links")
             elif isinstance(target, NXlink):
                 raise NeXusError("Cannot link to another NXlink object")
             if name is None: