Merge tag 'upstream/0.4.0' into master

Upstream version 0.4.0

23 files changed, 2238 insertions, 1413 deletions

diff --git a/DESCRIPTION b/DESCRIPTION
index 995cfc4..d4c7846 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,6 +1,6 @@
 Name: dicom
-Version: 0.3.0
-Date: 2020-02-27
+Version: 0.4.0
+Date: 2020-09-04
 Author: Andy Buckle
 Maintainer: Andy Buckle, John Donoghue
 Title: dicom: file io for medical images and other data
diff --git a/INDEX b/INDEX
index 219d124..f4a3a2b 100644
--- a/INDEX
+++ b/INDEX
@@ -1,5 +1,6 @@
 dicom >> Dicom file I/O for medical images and other data
 Dicom Functions
+  dicomanon
   dicomdict
   dicomdisp
   dicominfo
diff --git a/Makefile b/Makefile
index 642af95..782cc4d 100644
--- a/Makefile
+++ b/Makefile
@@ -75,7 +75,7 @@ doc/$(PACKAGE).pdf: doc/$(PACKAGE).texi doc/functions.texi
 	cd doc && $(RM) -f $(PACKAGE).aux $(PACKAGE).cp $(PACKAGE).cps $(PACKAGE).fn  $(PACKAGE).fns $(PACKAGE).log $(PACKAGE).toc
 
 doc/functions.texi:
-	cd doc && ./mkfuncdocs.py --src-dir=../inst/ --src-dir=../src/ ../INDEX > functions.texi
+	cd doc && ./mkfuncdocs.py --src-dir=../inst/ --src-dir=../src/ ../INDEX | $(SED) 's/@seealso/@xseealso/g' > functions.texi
 
 
 $(RELEASE_DIR): .hg/dirstate
diff --git a/NEWS b/NEWS
index 1e42e41..435a93e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,12 @@
+ Summary of important user-visible changes for dicom 0.4.0 (2020/09/04):
+-------------------------------------------------------------------------
+
+ ** bugfix dicomdisp, dicominfo for empty SQ sequences
+
+ ** added dicomanon function
+
+ ** minor document updates
+
  Summary of important user-visible changes for dicom 0.3.0 (2020/02/27):
 -------------------------------------------------------------------------
 
diff --git a/doc/dicom.pdf b/doc/dicom.pdf
index 9bd3673..0b25dde 100644
--- a/doc/dicom.pdf
+++ b/doc/dicom.pdf
diff --git a/doc/dicom.texi b/doc/dicom.texi
index 360944f..bae446a 100644
--- a/doc/dicom.texi
+++ b/doc/dicom.texi
@@ -11,29 +11,12 @@
 @afourpaper
 @paragraphindent 0
 @finalout
-@set VERSION 0.3.0
+@set VERSION 0.4.0
 @set COPYRIGHT_DATE 2019-2020
 @c @afourwide
 @c %*** End of the HEADER
 
-@c The following macro is used for the on-line help system, but we don't
-@c want lots of `See also: foo, bar, and baz' strings cluttering the
-@c printed manual (that information should be in the supporting text for
-@c each group of functions and variables).
-
-@macro seealso {args}
-@iftex
-@vskip 2pt
-@end iftex
-@ifnottex
-@c Texinfo @sp should work but in practice produces ugly results for HTML.
-@c A simple blank line produces the correct behavior.
-@c @sp 1
-
-@end ifnottex
-@noindent
-@strong{See also:} \args\.
-@end macro
+@include macros.texi
 
 @c %*** Start of TITLEPAGE
 @titlepage
diff --git a/doc/functions.texi b/doc/functions.texi
index 6578d28..1286a14 100644
--- a/doc/functions.texi
+++ b/doc/functions.texi
@@ -2,6 +2,30 @@
 @node  Dicom Functions
 @section  Dicom Functions
 @cindex  Dicom Functions
+@c Dicom Functions dicomanon
+@c -----------------------------------------
+@subsection  dicomanon
+@cindex  dicomanon
+@deftypefn {Loadable Function} {} dicomanon(@var{file_in}, @var{file_out})
+@deftypefnx {Loadable Function} {} dicomanon(___, @var{name}, @var{value})
+
+Anonymize a DICOM format file by removing or replacing specific fields.
+
+@var{file_in} is filename to read from.@*
+@var{file_out} is the filename to write to.@*
+@var{name}, @var{value} optional name/value properties.@*
+
+Known property names are:
+@table @asis
+@item keep
+The value is a cell array of names to not remove during the anonymize procedure.
+@item update
+A structure of name/values to update rather than remove.
+@end table
+
+@xseealso{dicomread, dicomwrite, dicominfo}
+@end deftypefn 
+
 @c Dicom Functions dicomdict
 @c -----------------------------------------
 @subsection  dicomdict
@@ -17,50 +41,50 @@ Using @code{factory} resets the dictionary to the default.
 Using @code{set} allows setting the dictionary for future operations.
 In this case, the dictionary file @var{dictionary_name} can be anywhere in the path.
 
-@seealso{dicomread, dicomwrite}
+@xseealso{dicomread, dicomwrite}
 @end deftypefn 
 
 @c Dicom Functions dicomdisp
 @c -----------------------------------------
 @subsection  dicomdisp
 @cindex  dicomdisp
- @deftypefn {Loadable Function} {} dicomdisp (@var{filename}) 
- @deftypefnx {Loadable Function} {} dicomdisp (@var{filename}, [@var{propertyname}, @var{propertvalue} ...]) 
- Read and display the metadata from a DICOM file.
- 
- @var{filename} - dicomfilename to display.@*
- @var{propertyname}, @var{propertvalue} - property pairs for options to the display function.
- 
- Currently the only known property is 'dictionary' to specify a non default dict to use.
- @seealso{dicomread, dicominfo} 
- @end deftypefn
- 
+@deftypefn {Loadable Function} {} dicomdisp (@var{filename}) 
+@deftypefnx {Loadable Function} {} dicomdisp (@var{filename}, [@var{propertyname}, @var{propertvalue} ...]) 
+Read and display the metadata from a DICOM file.
+
+@var{filename} - dicomfilename to display.@*
+@var{propertyname}, @var{propertvalue} - property pairs for options to the display function.
+
+Currently the only known property is 'dictionary' to specify a non default dict to use.
+@xseealso{dicomread, dicominfo} 
+@end deftypefn
+
 @c Dicom Functions dicominfo
 @c -----------------------------------------
 @subsection  dicominfo
 @cindex  dicominfo
- @deftypefn {Loadable Function} {@var{info}} = dicominfo (@var{filename}) 
- @deftypefnx {Loadable Function} {@var{info}} = dicominfo (@var{filename}, @code{dictionary}, @var{dictionary-name}) 
- @deftypefnx  {Loadable Function} {} dicominfo (@var{filename}, @var{options}) 
- @deftypefnx {Command} {} dicominfo @var{filename} 
- @deftypefnx {Command} {} dicominfo @var{filename} @var{options} 
- Get all data from a DICOM file, excluding any actual image. 
- @var{info} is a nested struct containing the data. 
- 
- If no return argument is given, then there will be output similar to 
- a DICOM dump. 
- 
- If the @code{dictionary} argument is used, the given @var{dictionary-name} is used for this operation, 
- otherwise, the dictionary set by @code{dicomdict} is used.
- 
- @var{options}:
- @code{truncate=n}
- where n is the number of characters to limit the dump output display to @code{n}
- for each value. 
-
- @seealso{dicomread, dicomdict} 
- @end deftypefn
- 
+@deftypefn {Loadable Function} {@var{info}} = dicominfo (@var{filename}) 
+@deftypefnx {Loadable Function} {@var{info}} = dicominfo (@var{filename}, @code{dictionary}, @var{dictionary-name}) 
+@deftypefnx  {Loadable Function} {} dicominfo (@var{filename}, @var{options}) 
+@deftypefnx {Command} {} dicominfo @var{filename} 
+@deftypefnx {Command} {} dicominfo @var{filename} @var{options} 
+Get all data from a DICOM file, excluding any actual image. 
+@var{info} is a nested struct containing the data. 
+
+If no return argument is given, then there will be output similar to 
+a DICOM dump. 
+
+If the @code{dictionary} argument is used, the given @var{dictionary-name} is used for this operation, 
+otherwise, the dictionary set by @code{dicomdict} is used.
+
+@var{options}:
+@code{truncate=n}
+where n is the number of characters to limit the dump output display to @code{n}
+for each value. 
+
+@xseealso{dicomread, dicomdict} 
+@end deftypefn
+
 @c Dicom Functions dicomlookup
 @c -----------------------------------------
 @subsection  dicomlookup
@@ -78,25 +102,25 @@ of the attribute.
 dictionary for a specified @var{keyword} string and returns the @var{group} and @var{element}
 for keyword.
 
-@seealso{dicomdict}
+@xseealso{dicomdict}
 @end deftypefn 
 
 @c Dicom Functions dicomread
 @c -----------------------------------------
 @subsection  dicomread
 @cindex  dicomread
- @deftypefn {Loadable Function} @var{image} = dicomread (@var{filename}) 
- @deftypefnx {Loadable Function} @var{image} = dicomread (@var{structure}) 
- 
- Load the image from a DICOM file. 
- @var{filename} is a string (giving the filename).
- @var{structure} is a structure with a field @code{Filename} (such as returned by @code{dicominfo}).
- @var{image} may be two or three dimensional, depending on the content of the file. 
- An integer or float matrix will be returned, the number of bits will depend on the file. 
-
- @seealso{dicominfo} 
- @end deftypefn 
- 
+@deftypefn {Loadable Function} @var{image} = dicomread (@var{filename}) 
+@deftypefnx {Loadable Function} @var{image} = dicomread (@var{structure}) 
+
+Load the image from a DICOM file. 
+@var{filename} is a string (giving the filename).
+@var{structure} is a structure with a field @code{Filename} (such as returned by @code{dicominfo}).
+@var{image} may be two or three dimensional, depending on the content of the file. 
+An integer or float matrix will be returned, the number of bits will depend on the file. 
+
+@xseealso{dicominfo} 
+@end deftypefn 
+
 @c Dicom Functions dicomuid
 @c -----------------------------------------
 @subsection  dicomuid
@@ -122,7 +146,7 @@ Write a DICOM format file to @var{filename}.
 @var{filename} is filename to write dicom to. if [], then function runs in verbose trial mode.
 @var{info} struct, like that produced by dicominfo
 
-@seealso{dicomread, dicominfo}
+@xseealso{dicomread, dicominfo}
 @end deftypefn 
 
 @c Dicom Functions isdicom
@@ -132,5 +156,5 @@ Write a DICOM format file to @var{filename}.
 @deftypefn {Loadable Function} {} isdicom (@var{filename}) 
 Return true if @var{filename} is a valid DICOM file.
 
-@seealso{dicomdict, dicominfo, dicomread, dicomwrite} 
+@xseealso{dicomdict, dicominfo, dicomread, dicomwrite} 
 @end deftypefn
diff --git a/doc/macros.texi b/doc/macros.texi
new file mode 100644
index 0000000..291bcc4
--- /dev/null
+++ b/doc/macros.texi
@@ -0,0 +1,117 @@
+@c Copyright (C) 2012-2019 John W. Eaton
+@c
+@c This file is part of Octave.
+@c
+@c Octave is free software: you can redistribute it and/or modify it
+@c under the terms of the GNU General Public License as published by
+@c the Free Software Foundation, either version 3 of the License, or
+@c (at your option) any later version.
+@c
+@c Octave is distributed in the hope that it will be useful, but
+@c WITHOUT ANY WARRANTY; without even the implied warranty of
+@c MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+@c GNU General Public License for more details.
+@c
+@c You should have received a copy of the GNU General Public License
+@c along with Octave; see the file COPYING.  If not, see
+@c <https://www.gnu.org/licenses/>.
+
+@c The following macro marks words that aspell should ignore during
+@c spellchecking.  Within Texinfo it has no effect as it merely replaces
+@c the macro call with the argument itself.
+
+@macro nospell {arg}
+\arg\
+@end macro
+
+@c The following macro works around the Info/plain text expansion of @code{XXX}
+@c which is `XXX'.  This looks particularly bad when the macro body is
+@c single or double-quoted text, such as a property value `"position"'
+@ifinfo
+@macro qcode{arg}
+\arg\
+@end macro
+@end ifinfo
+@ifnotinfo
+@macro qcode{arg}
+@code{\arg\}
+@end macro
+@end ifnotinfo
+
+@c The following macro is used for the on-line help system, but we don't
+@c want lots of `See also: foo, bar, and baz' strings cluttering the
+@c printed manual (that information should be in the supporting text for
+@c each group of functions and variables).
+@c
+@c Implementation Note:
+@c For TeX, @vskip produces a nice separation.
+@c For Texinfo, '@sp 1' should work, but in practice produces ugly results
+@c for HTML.  We use a simple blank line to produce the correct
+@c behavior.
+@c
+@c We use @xseealso now because Texinfo introduced its own @seealso
+@c command.  But instead of modifying all source files, we'll have the
+@c munge-texi script convert @seealso to @xseealso.
+
+@macro xseealso {args}
+@iftex
+@vskip 2pt
+@end iftex
+@ifnottex
+
+@end ifnottex
+@ifnotinfo
+@noindent
+@strong{See also:} \args\.
+@end ifnotinfo
+@ifinfo
+@noindent
+See also: \args\.
+@end ifinfo
+@end macro
+
+@c The following macro works around a situation where the Info/plain text
+@c expansion of the @code{XXX} macro is `XXX'.  The use of the apostrophe
+@c can be confusing if the code segment itself ends with a transpose operator.
+@ifinfo
+@macro tcode{arg}
+\arg\
+@end macro
+@end ifinfo
+@ifnotinfo
+@macro tcode{arg}
+@code{\arg\}
+@end macro
+@end ifnotinfo
+
+@c FIXME: someday, when Texinfo 5.X is standard, we might replace this with
+@c @backslashchar, which is a new addition to Texinfo.
+
+@macro xbackslashchar
+\\
+@end macro
+
+@c These may be useful for all, not just for octave.texi.
+@tex
+  \ifx\rgbDarkRed\thisisundefined
+    \def\rgbDarkRed{0.50 0.09 0.12}
+  \fi
+  \ifx\linkcolor\thisisundefined
+    \relax
+  \else
+    \global\def\linkcolor{\rgbDarkRed}
+  \fi
+  \ifx\urlcolor\thisisundefined
+    \relax
+  \else
+    \global\def\urlcolor{\rgbDarkRed}
+  \fi
+  \ifx\urefurlonlylinktrue\thisisundefined
+    \relax
+  \else
+    \global\urefurlonlylinktrue
+  \fi
+@end tex
+
+@c Make the apostrophe in code examples cut-and-paste friendly.
+@codequoteundirected on
diff --git a/doc/mkfuncdocs.py b/doc/mkfuncdocs.py
index f140896..f5b99bd 100755
--- a/doc/mkfuncdocs.py
+++ b/doc/mkfuncdocs.py
@@ -1,6 +1,6 @@
 #!/usr/bin/python2
 
-## Copyright 2018-2019 John Donoghue
+## Copyright 2018-2020 John Donoghue
 ##
 ## This program is free software: you can redistribute it and/or modify it
 ## under the terms of the GNU General Public License as published by
@@ -16,7 +16,8 @@
 ## along with this program.  If not, see
 ## <https://www.gnu.org/licenses/>.
 
-## mkfuncdocs.py will atempt to extract the help texts from functions in src
+## mkfuncdocs v1.0.1
+## mkfuncdocs.py will attempt to extract the help texts from functions in src
 ## dirs, extracting only those that are in the specifed INDEX file and output them
 ## to stdout in texi format
 ##
@@ -37,6 +38,7 @@
 ##   --ignore=xxxxx  : dont attempt to generate help for function xxxxx.
 ##   --funcprefix=xxxxx : remove xxxxx from the function name when searching for matching
 ##                     source file.
+##   --allowscan     : if can not find function, attemp to scan .cc,cpp,cxx files for match
 
 import sys
 import os
@@ -61,13 +63,29 @@ class Index:
   name = ""
   groups = []
 
-def read_m_file(filename):
+def find_defun_line_in_file(filename, fnname):
+  linecnt = 0
+
+  defun_line=re.compile("^DEFUN_DLD\s*\(\s*{}".format(fnname))
+  with open(filename, 'rt') as f:
+    for line in f:
+      if re.match(defun_line, line):
+        return linecnt
+
+      linecnt = linecnt + 1
+
+  return -1
+
+def read_m_file(filename, skip=0):
   help = []
   inhelp = False
   havehelp = False;
   with open(filename, 'rt') as f:
     for line in f:
-      if not havehelp:
+      line = line.lstrip()
+      if skip > 0:
+        skip = skip - 1
+      elif not havehelp:
         if havehelp == False and inhelp == False and line.startswith('##'):
           if "texinfo" in line:
             inhelp = True
@@ -80,13 +98,16 @@ def read_m_file(filename):
 
   return help
 
-def read_cc_file(filename):
+def read_cc_file(filename, skip=0):
   help = []
   inhelp = False
   havehelp = False;
   with open(filename, 'rt') as f:
     for line in f:
-      if not havehelp:
+      line = line.lstrip()
+      if skip > 0:
+        skip = skip - 1
+      elif not havehelp:
         if havehelp == False and inhelp == False:
           if "texinfo" in line:
             inhelp = True
@@ -112,13 +133,13 @@ def read_cc_file(filename):
 
   return help
 
-def read_help (filename):
+def read_help (filename, skip=0):
   help = []
 
   if filename[-2:] == ".m":
-    help = read_m_file(filename)
+    help = read_m_file(filename, skip)
   else:
-    help = read_cc_file(filename)
+    help = read_cc_file(filename, skip)
 
   return help
 
@@ -156,28 +177,44 @@ def read_index (filename, ignore):
 
   return index;
 
-def find_func_file(fname, paths, prefix):
+def find_func_file(fname, paths, prefix, scanfiles=False):
   for f in paths:
       name = f + "/" + fname + ".m"
       if os.path.isfile(name):
-        return name
+        return name, 0
       name = f + "/" + fname + ".cc"
       if os.path.isfile(name):
-        return name
+        return name, 0
       name = f + "/" + fname + ".cpp"
       if os.path.isfile(name):
-        return name
+        return name, 0
       # if have a prefix, remove and try
       if prefix and fname.startswith(prefix):
         fname = fname[len(prefix):]
         name = f + "/" + fname + ".cc"
         if os.path.isfile(name):
-          return name
+          return name, 0
         name = f + "/" + fname + ".cpp"
         if os.path.isfile(name):
-          return name
- 
-  return None
+          return name, 0
+
+  # if here, we still dont have a file match
+  # if allowed to scan files, do that
+  if scanfiles:
+    #sys.stderr.write("Warning: Scaning for {}\n".format(fname))
+    for f in paths:
+      files = list(f + "/" + a for a in os.listdir(f))
+      cc_files = fnmatch.filter(files, "*.cc")
+      cpp_files = fnmatch.filter(files, "*.cpp")
+      cxx_files = fnmatch.filter(files, "*.cxx")
+
+      for fn in cc_files + cpp_files + cxx_files:
+        line = find_defun_line_in_file(fn, fname)
+        if line >= 0:
+          #sys.stderr.write("Warning: Found function for {} in {} at {}\n".format(fname, fn, line))
+          return fn, line
+  
+  return None, -1
 
 def display_func(name, ref, help):
   print "@c -----------------------------------------"
@@ -187,7 +224,7 @@ def display_func(name, ref, help):
     print l
 
 def process (args):
-  options = { "verbose": False, "srcdir": [], "funcprefix": "", "ignore": [] }
+  options = { "verbose": False, "srcdir": [], "funcprefix": "", "ignore": [], "allowscan": False }
   indexfile = ""
 
   for a in args:
@@ -202,6 +239,8 @@ def process (args):
 
     if key == "--verbose":
       options["verbose"] = True;
+    elif key == "--allowscan":
+      options["allowscan"] = True;
     elif key == "--src-dir":
       if val:
         options["srcdir"].append(val);
@@ -241,7 +280,7 @@ def process (args):
         path = f
         name = "@" + f
         ref = f.split("/")[-1]
-        filename = find_func_file(path, options["srcdir"], options["funcprefix"])
+        filename, lineno = find_func_file(path, options["srcdir"], options["funcprefix"])
       elif "." in f:
         parts = f.split('.')
         cnt  = 0
@@ -255,22 +294,22 @@ def process (args):
             cnt = cnt + 1
         name = f;
         ref = parts[-1]
-        filename = find_func_file(path, options["srcdir"], options["funcprefix"])
+        filename, lineno = find_func_file(path, options["srcdir"], options["funcprefix"])
       elif "/" in f:
         path = f
         name = f
         ref = f.split("/")[-1]
-        filename = find_func_file(path, options["srcdir"], options["funcprefix"])
+        filename, lineno = find_func_file(path, options["srcdir"], options["funcprefix"])
       else:
         path = f
         name = f
         ref = f
-        filename = find_func_file(path, options["srcdir"], options["funcprefix"])
+        filename, lineno = find_func_file(path, options["srcdir"], options["funcprefix"], options['allowscan'])
 
       if not filename:
         sys.stderr.write("Warning: Cant find source file for {}\n".format(path))
       else:
-        h = read_help (filename)
+        h = read_help (filename, lineno)
 
       if h:
         display_func (name, ref, h)
diff --git a/src/Makefile.in b/src/Makefile.in
index 65d3d41..2aa221b 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -7,9 +7,9 @@ ARCHDIR   := "$(shell $(OCTAVE_CONFIG) -p CANONICAL_HOST_TYPE)-$(shell $(OCTAVE_
 GDCM_CPPFLAGS = @GDCM_CXXFLAGS@ @DEFS@
 GDCM_LIBS = @GDCM_LIBS@
 
-need_dict = dicominfo.oct dicomwrite.oct dicomlookup.oct dicomdisp.oct
+need_dict = dicominfo.oct dicomwrite.oct dicomlookup.oct dicomdisp.oct dicomanon.oct
 
-test_files = dicominfo.cpp dicomdict.cpp dicomread.cpp dicomlookup.cpp isdicom.cpp dicomuid.cpp dicomdisp.cpp
+test_files = dicominfo.cpp dicomdict.cpp dicomread.cpp dicomlookup.cpp isdicom.cpp dicomuid.cpp dicomdisp.cpp dicomanon.cpp
 
 all: $(need_dict) dicomdict.oct dicomread.oct _gendicomdict.oct isdicom.oct dicomuid.oct archtests
 
diff --git a/src/_gendicomdict.cpp b/src/_gendicomdict.cpp
index dce19ed..ad19c7d 100644
--- a/src/_gendicomdict.cpp
+++ b/src/_gendicomdict.cpp
@@ -44,70 +44,83 @@
 #define QUOTED_(x) #x
 #define QUOTED(x) QUOTED_(x)
 
-char* name2Keyword(char *d, int *d_len_p, const char* s);
+char* name2Keyword (char *d, int *d_len_p, const char* s);
 
 // This takes around 70 min to run on my laptop
 // This is a very dirty way to make a dictionary
 // It is transparently independent of Matlab
 // TODO: needs to make tags with xx in that give ranges
 DEFUN_DLD (OCT_FN_NAME, args, nargout,
-		"extract data from gdcm libs to make a dict for octave") {
-	octave_value_list retval;  // create object to store return values
-	if (args.length () != 0) {
-		error(QUOTED(OCT_FN_NAME)": no arguments required, got %i.", (int)args.length ());
-		return retval; 
-	}
-	
-	// get dicom dictionary
-	const gdcm::Global &g = gdcm::Global::GetInstance();
-	const gdcm::Dicts &dicts = g.GetDicts();
-	const char *strowner = 0;
-	
-	uint16_t gi, ei;
-	
-	int buflen=32;
-	char * keybuf=(char *)malloc(buflen*sizeof(char));
-	
-	// TODO: use Keywords instead of names
-	// TODO: option to write to file instead of terminal
-	std::ofstream dic;
-	dic.open("octavedicom.dic"); //TODO: check for IO problems
-	dic << std::resetiosflags(std::ios_base::showbase);
-	dic << std::setiosflags(std::ios::uppercase) ;
-	dic << std::setbase(16) << std::setfill('0');
-	octave_stdout.precision(2);
-	for(gi=0x0; gi<0xFFFF; gi++) {
-		if(0== gi%64) {
-			octave_stdout << ((double)gi)/((double)0xFFFF) << "  " ; //progress
-		}
-		for(ei=0x1; ei<0xFFFF; ei++) { // ei starts at 1. 0 is group length tags
-			const gdcm::Tag tag(gi, ei);
-			if (tag.IsIllegal()) continue;
-			const gdcm::DictEntry dictEntry = dicts.GetDictEntry(tag,strowner) ;
-			if(gdcm::VR::INVALID==dictEntry.GetVR()) continue ;
-			if(!strcmp("Private Creator",dictEntry.GetName())) continue ; //TODO: for these dicominfo will do something...
-			// gdcm::DictEntry::GetKeyword() seems to always return ""
-			if (strlen(dictEntry.GetName()) == 0) continue;
-			std::stringstream ss;
-			ss << dictEntry.GetVR()  ;
-			if (' '==ss.str()[2]) { // change "OB or OW" to "OB/OW"
-				ss.str(ss.str().substr(0,2)+'/'+ss.str().substr(6,2));
-			}
-			const char *tagName=dictEntry.GetName();
-			keybuf=name2Keyword(keybuf,&buflen,tagName);
-			dic << '(' << std::setw(4) << gi ;
-			dic << std::setw(1) << "," ;
-			dic << std::setw(4) << ei;
-			dic << std::setw(1) << ")\t" ;
-			dic << ss.str() << '\t' ;
-			dic << keybuf << '\t' ;
-			dic << dictEntry.GetVM() << '\n' ;
-		}
-	}
-	dic.close();
-	free(keybuf);
-	octave_stdout << '\n' ;
-	return retval;
+  "extract data from gdcm libs to make a dict for octave")
+{
+  octave_value_list retval;  // create object to store return values
+  if (args.length () != 0)
+    {
+      error (QUOTED(OCT_FN_NAME)": no arguments required, got %i.", (int)args.length ());
+      return retval; 
+    }
+  
+  // get dicom dictionary
+  const gdcm::Global &g = gdcm::Global::GetInstance();
+  const gdcm::Dicts &dicts = g.GetDicts();
+  const char *strowner = 0;
+  
+  uint16_t gi, ei;
+  
+  int buflen=32;
+  char * keybuf = (char *)malloc(buflen*sizeof(char));
+  
+  // TODO: use Keywords instead of names
+  // TODO: option to write to file instead of terminal
+  std::ofstream dic;
+  dic.open("octavedicom.dic"); //TODO: check for IO problems
+  dic << std::resetiosflags(std::ios_base::showbase);
+  dic << std::setiosflags(std::ios::uppercase) ;
+  dic << std::setbase(16) << std::setfill('0');
+  octave_stdout.precision(2);
+  for (gi = 0x0; gi < 0xFFFF; gi++)
+    {
+      if (0 == gi%64)
+        {
+          octave_stdout << ((double)gi)/((double)0xFFFF) << "  " ; //progress
+        }
+      for (ei = 0x1; ei < 0xFFFF; ei++)
+        { 
+          // ei starts at 1. 0 is group length tags
+          const gdcm::Tag tag(gi, ei);
+          if (tag.IsIllegal())
+            continue;
+          const gdcm::DictEntry dictEntry = dicts.GetDictEntry(tag,strowner) ;
+          if (gdcm::VR::INVALID==dictEntry.GetVR())
+            continue;
+          //TODO: for these dicominfo will do something...
+          if (! strcmp("Private Creator",dictEntry.GetName())) 
+            continue;
+          // gdcm::DictEntry::GetKeyword() seems to always return ""
+          if (strlen(dictEntry.GetName()) == 0)
+            continue;
+          std::stringstream ss;
+          ss << dictEntry.GetVR();
+          if (' ' == ss.str()[2]) 
+            { 
+              // change "OB or OW" to "OB/OW"
+              ss.str(ss.str().substr(0,2)+'/'+ss.str().substr(6,2));
+            }
+          const char *tagName = dictEntry.GetName();
+          keybuf = name2Keyword(keybuf,&buflen,tagName);
+          dic << '(' << std::setw(4) << gi;
+          dic << std::setw(1) << ",";
+          dic << std::setw(4) << ei;
+          dic << std::setw(1) << ")\t";
+          dic << ss.str() << '\t';
+          dic << keybuf << '\t';
+          dic << dictEntry.GetVM() << '\n';
+        }
+    }
+  dic.close();
+  free(keybuf);
+  octave_stdout << '\n';
+  return retval;
 }
 
 // remove non-alphabet characters from a string.
@@ -116,22 +129,30 @@ DEFUN_DLD (OCT_FN_NAME, args, nargout,
 // this fn will realloc if it is not big enough. so use
 // returned pointer, as the supplied one may be invalid.
 // d_len_p: pointer to length of d. is updated if required.
-char* name2Keyword(char *d, int *d_len_p, const char* s) {
-	char *f=(char*)s; //from (loop through source)
-	int len=strlen(s);
-	if ( len > *d_len_p ) {
-		d=(char *)realloc(d,(len+1)*sizeof(char));
-	}
-	char *tl=(char*)d; // pointer to loop through the destination
-	for (; *f != '\0' ; f++ ) {
-		if ( (*f >= 'A' && *f <= 'Z') || (*f >= 'a' && *f <= 'z') ) {
-			*tl++ = *f;
-		} else if (*f=='\'' && *(f+1)=='s') {
-			f++; // if quote followed by s, skip both chars
-		} else if (*f==' ' && *(f+1) >= 'a' && *(f+1) <= 'z') {
-			*tl++ = *++f - ('a'-'A') ; // space folowed by lower case char, cap char
-		}
-	}
-	*tl = '\0';
-	return d;
+char* name2Keyword (char *d, int *d_len_p, const char* s)
+{
+  char *f = (char*)s; //from (loop through source)
+  int len=strlen(s);
+  if (len > *d_len_p )
+    {
+      d = (char *)realloc (d, (len+1)*sizeof(char));
+    }
+  char *tl = (char*)d; // pointer to loop through the destination
+  for (; *f != '\0' ; f++ )
+    {
+      if ( (*f >= 'A' && *f <= 'Z') || (*f >= 'a' && *f <= 'z') )
+        {
+          *tl++ = *f;
+        }
+      else if (*f =='\'' && *(f+1) == 's')
+        {
+          f++; // if quote followed by s, skip both chars
+        }
+      else if (*f ==' ' && *(f+1) >= 'a' && *(f+1) <= 'z')
+        {
+          *tl++ = *++f - ('a'-'A') ; // space folowed by lower case char, cap char
+        }
+    }
+  *tl = '\0';
+  return d;
 }
diff --git a/src/config.h.in b/src/config.h.in
index 70a8145..57d0f41 100644
--- a/src/config.h.in
+++ b/src/config.h.in
@@ -39,6 +39,9 @@
 #undef OCTAVE__VALID_IDENTIFIER
 
 /* macro for alternative Octave symbols */
+#undef OV_ISCELL
+
+/* macro for alternative Octave symbols */
 #undef OV_ISMAP
 
 /* Define to the address where bug reports for this package should be sent. */
diff --git a/src/configure b/src/configure
index adb4949..7453bb1 100755
--- a/src/configure
+++ b/src/configure
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for Octave-Forge dicom package 0.3.0.
+# Generated by GNU Autoconf 2.69 for Octave-Forge dicom package 0.4.0.
 #
 #
 # Copyright (C) 1992-1996, 1998-2012 Free Software Foundation, Inc.
@@ -577,8 +577,8 @@ MAKEFLAGS=
 # Identity of this package.
 PACKAGE_NAME='Octave-Forge dicom package'
 PACKAGE_TARNAME='octave-forge-dicom-package'
-PACKAGE_VERSION='0.3.0'
-PACKAGE_STRING='Octave-Forge dicom package 0.3.0'
+PACKAGE_VERSION='0.4.0'
+PACKAGE_STRING='Octave-Forge dicom package 0.4.0'
 PACKAGE_BUGREPORT=''
 PACKAGE_URL=''
 
@@ -669,7 +669,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -744,7 +743,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -997,15 +995,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1143,7 +1132,7 @@ fi
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1256,7 +1245,7 @@ if test "$ac_init_help" = "long"; then
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures Octave-Forge dicom package 0.3.0 to adapt to many kinds of systems.
+\`configure' configures Octave-Forge dicom package 0.4.0 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1296,7 +1285,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
@@ -1324,7 +1312,7 @@ fi
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of Octave-Forge dicom package 0.3.0:";;
+     short | recursive ) echo "Configuration of Octave-Forge dicom package 0.4.0:";;
    esac
   cat <<\_ACEOF
 
@@ -1409,7 +1397,7 @@ fi
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-Octave-Forge dicom package configure 0.3.0
+Octave-Forge dicom package configure 0.4.0
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -1707,7 +1695,7 @@ cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by Octave-Forge dicom package $as_me 0.3.0, which was
+It was created by Octave-Forge dicom package $as_me 0.4.0, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -3100,6 +3088,40 @@ $as_echo "  is_map" >&6; }
 fi
 rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking   is_cell or iscell" >&5
+$as_echo_n "checking   is_cell or iscell... " >&6; }
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <octave/oct.h>
+
+
+int
+main ()
+{
+octave_value ().iscell ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+
+$as_echo "#define OV_ISCELL iscell" >>confdefs.h
+
+     { $as_echo "$as_me:${as_lineno-$LINENO}: result: iscell" >&5
+$as_echo "iscell" >&6; }
+     echo '
+' >> oct-alt-includes.h
+else
+
+$as_echo "#define OV_ISCELL   is_cell" >>confdefs.h
+
+     { $as_echo "$as_me:${as_lineno-$LINENO}: result:   is_cell" >&5
+$as_echo "  is_cell" >&6; }
+     echo '' >> oct-alt-includes.h
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking   valid_identifier or octave::valid_identifier" >&5
 $as_echo_n "checking   valid_identifier or octave::valid_identifier... " >&6; }
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -4265,7 +4287,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by Octave-Forge dicom package $as_me 0.3.0, which was
+This file was extended by Octave-Forge dicom package $as_me 0.4.0, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -4327,7 +4349,7 @@ _ACEOF
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
-Octave-Forge dicom package config.status 0.3.0
+Octave-Forge dicom package config.status 0.4.0
 configured by $0, generated by GNU Autoconf 2.69,
   with options \\"\$ac_cs_config\\"
 
diff --git a/src/configure.ac b/src/configure.ac
index 51d9d5b..19dde87 100644
--- a/src/configure.ac
+++ b/src/configure.ac
@@ -18,7 +18,7 @@
 ### <http://www.gnu.org/licenses/>.
 
 AC_PREREQ([2.67])
-AC_INIT([Octave-Forge dicom package], [0.3.0])
+AC_INIT([Octave-Forge dicom package], [0.4.0])
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIRS([m4])
 AH_TOP([#include "undef-ah-octave.h"])
@@ -89,6 +89,15 @@ OF_OCTAVE_LIST_ALT_SYMS([
 ],
 
 [dnl
+  [is_cell],
+  [iscell],
+  [[octave_value ().iscell ();]],
+  [OV_ISCELL],
+  [],
+  []
+],
+
+[dnl
   [valid_identifier],
   [octave::valid_identifier],
   [[octave::valid_identifier("");]],
diff --git a/src/dicomanon.cpp b/src/dicomanon.cpp
new file mode 100644
index 0000000..a948d96
--- /dev/null
+++ b/src/dicomanon.cpp
@@ -0,0 +1,276 @@
+/*
+ * Copyright John Donoghue, 2020
+ * 
+ * The GNU Octave dicom package is free software: you can redistribute 
+ * it and/or modify it under the terms of the GNU General Public 
+ * License as published by the Free Software Foundation, either 
+ * version 3 of the License, or (at your option) any later version.
+ * 
+ * The GNU Octave dicom packag is distributed in the hope that it 
+ * will be useful, but WITHOUT ANY WARRANTY; without even the 
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR 
+ * PURPOSE.  See the GNU General Public License for more details.
+ * 
+ * Please see the file, "COPYING" for further details of GNU General 
+ * Public License version 3.
+ * 
+ */
+
+#include <octave/oct.h>
+#include <octave/ov-struct.h>
+#include <octave/Cell.h>
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <gdcmReader.h>
+#include <gdcmWriter.h>
+#include <gdcmAnonymizer.h>
+#include <gdcmDictEntry.h>
+#include <gdcmDefs.h>
+#include <gdcmUIDGenerator.h>
+
+#include "dicomdict.h"
+              
+#define OCT_FN_NAME dicomanon
+#define QUOTED_(x) #x
+#define QUOTED(x) QUOTED_(x)
+
+bool lookup_tag (const std::string & keyword, gdcm::Tag &tag)
+{
+  gdcm::DictEntry entry;
+  if (!dicom_is_present(keyword))
+    {
+      return false;
+    }
+  lookup_dicom_tag (tag, keyword);
+  lookup_dicom_entry (entry, tag);
+  return true;
+}
+
+DEFUN_DLD (dicomanon, args, nargout,
+    "-*- texinfo -*- \n\
+@deftypefn {Loadable Function} {} dicomanon(@var{file_in}, @var{file_out})\n\
+@deftypefnx {Loadable Function} {} dicomanon(___, @var{name}, @var{value})\n\
+\n\
+Anonymize a DICOM format file by removing or replacing specific fields.\n\
+\n\
+@var{file_in} is filename to read from.@*\n\
+@var{file_out} is the filename to write to.@*\n\
+@var{name}, @var{value} optional name/value properties.@*\n\
+\n\
+Known property names are:\n\
+@table @asis\n\
+@item keep\n\
+The value is a cell array of names to not remove during the anonymize procedure.\n\
+@item update\n\
+A structure of name/values to update rather than remove.\n\
+@end table\n\
+\n\
+@seealso{dicomread, dicomwrite, dicominfo}\n\
+@end deftypefn \n\
+")
+{
+  octave_value_list retval;  // create object to store return values
+  if (2 > args.length())
+    {
+      error (QUOTED(OCT_FN_NAME)": should have at least 2 arguments");
+      return retval; 
+    }
+
+  if (!  (args(0).is_string () && args(1).is_string ()))
+    {
+      error (QUOTED(OCT_FN_NAME)": first and second argument should be a string filename");
+      return retval; 
+    }
+
+  std::vector<gdcm::Tag> remove_tags = 
+    gdcm::Anonymizer::GetBasicApplicationLevelConfidentialityProfileAttributes ();
+
+  // remove (0x0008, 0x0018) as we MUST have it to create the file
+  remove(remove_tags.begin(), remove_tags.end(), gdcm::Tag(0x0008, 0x0018));
+
+  std::vector< std::pair<gdcm::Tag, std::string> > replace_tags;
+  gdcm::UIDGenerator uid;
+  const char *u = uid.Generate();
+  replace_tags.push_back(std::pair<gdcm::Tag, std::string>(gdcm::Tag(0x0008, 0x0018), u));
+
+  if (args.length() % 2 !=  0)
+    {
+      error (QUOTED(OCT_FN_NAME)": expected name/value pairs after filenames");
+      return retval; 
+    }
+
+  for (int i=2; i<args.length(); i+=2)
+    {
+      if (!  args(i).is_string ())
+        {
+          error (QUOTED(OCT_FN_NAME)": expected propty name at input %d", (i+1));
+          return retval; 
+        }
+
+      std::string name = args(i).string_value ();
+      if (name == "keep")
+        {
+          if (! args(i+1).OV_ISCELL())
+            {
+              error (QUOTED(OCT_FN_NAME)": expected fields value to be a cell aray");
+              return retval; 
+            }
+          Cell values = args(i+1).cell_value();
+          gdcm::Tag tag(0x0008, 0x0018);
+          for (octave_idx_type idx = 0; idx < values.numel (); idx++)
+            {
+              name = values(idx).string_value();
+              if (lookup_tag(name, tag))
+                {
+                  remove(remove_tags.begin(), remove_tags.end(), tag);
+                }
+            }
+        }
+      else if (name == "update")
+        {
+          if (! args(i+1).OV_ISMAP())
+            {
+              error (QUOTED(OCT_FN_NAME)": expected update value to be a struct");
+              return retval; 
+            }
+
+          octave_scalar_map values = args(i+1).scalar_map_value();
+          gdcm::Tag tag(0x0008, 0x0018);
+          for (octave_scalar_map::iterator it = values.begin(); it != values.end(); it++)
+            {
+              name = values.key(it);
+              
+	      octave_value value = values.contents(it);
+              if (lookup_tag(name, tag))
+                {
+                  remove(remove_tags.begin(), remove_tags.end(), tag);
+                  // add to update
+                  replace_tags.push_back(std::pair<gdcm::Tag, std::string>(tag, value.string_value()));
+                }
+            }
+        }
+      else
+        {
+          error (QUOTED(OCT_FN_NAME)": unknown property name '%s'", name.c_str());
+          return retval; 
+        }
+    }
+
+  std::string infilename = args(0).string_value ();
+  std::string outfilename = args(1).string_value ();
+
+  gdcm::Reader reader;
+  reader.SetFileName( infilename.c_str() );
+
+  if (!reader.Read ())
+    {
+      error (QUOTED(OCT_FN_NAME)": Could not read DICOM file: %s",infilename.c_str());
+      return retval;
+    }
+
+  gdcm::File &file = reader.GetFile();
+
+  gdcm::Anonymizer anon;
+  anon.SetFile( file );
+
+  // remove tags we want to remove
+  std::vector<gdcm::Tag>::const_iterator it = remove_tags.begin();
+
+  for (; it != remove_tags.end(); ++it)
+    {
+      gdcm::Tag tag = *it;
+      if (! anon.Remove (tag))
+        {
+          warning (QUOTED(OCT_FN_NAME)": Failed to remove: %04x %04x",tag.GetGroup(), tag.GetElement());
+        }
+    }
+
+  std::vector< std::pair<gdcm::Tag, std::string> >::const_iterator it2 = replace_tags.begin();
+
+  // update tages we want to update
+  for(; it2 != replace_tags.end(); ++it2)
+    {
+      gdcm::Tag tag = it2->first;
+      if (! anon.Replace (tag, it2->second.c_str()))
+        {
+          warning (QUOTED(OCT_FN_NAME)": Failed to replace: %04x %04x",tag.GetGroup(), tag.GetElement());
+        }
+    }
+
+  gdcm::FileMetaInformation &fmi = file.GetHeader();
+  fmi.Clear();
+
+  gdcm::Writer writer;
+
+  writer.SetFileName( outfilename.c_str() );
+  writer.SetFile( file );
+
+  if (! writer.Write())
+    {
+      error (QUOTED(OCT_FN_NAME)": Could not write DICOM file: %s", outfilename.c_str());
+
+      return retval;
+    }
+
+  return retval;
+}
+
+/*
+%!shared testfile1, testfile2
+%! testfile1 = tempname();
+%! testfile2 = tempname();
+%! wdata = uint8 (10*rand (10,10));
+%! s.PatientName = "John";
+%! s.PatientAge = "20";
+%! dicomwrite (wdata, testfile1, s);
+
+%!fail ("dicomanon");
+%!fail ("dicomanon (1, 1)");
+%!fail ("dicomanon (testfile1, 1)");
+%!fail ("dicomanon (testfile1, testfile2, 'a')");
+
+%!test
+%! info = dicominfo(testfile1);
+%! assert (isfield(info, "PatientName"));
+%! assert (isfield(info, "PatientAge"));
+%! assert (info.PatientName, "John");
+%! assert (info.PatientAge, "20");
+
+%!test
+%! dicomanon(testfile1, testfile2);
+%! info2 = dicominfo(testfile2);
+%! assert (!isfield(info2, "PatientName"));
+%! assert (!isfield(info2, "PatientAge"));
+
+%!test
+%! dicomanon(testfile1, testfile2, "keep", {"PatientAge"});
+%! info3 = dicominfo(testfile2);
+%! assert (isfield(info3, "PatientAge"));
+%! assert (info3.PatientAge, "20");
+%! assert (!isfield(info3, "PatientName"));
+
+%!test
+%! attrs.PatientAge = "21";
+%! dicomanon(testfile1, testfile2, "update", attrs);
+%! info5 = dicominfo(testfile2);
+%! assert (info5.PatientAge, "21");
+%! assert (!isfield(info5, "PatientName"));
+
+%!test
+%! attrs.PatientAge = "21";
+%! dicomanon(testfile1, testfile2, "update", attrs, "keep", {'PatientName'});
+%! info6 = dicominfo(testfile2);
+%! assert (info6.PatientAge, "21");
+%! assert (isfield(info6, "PatientName"));
+
+%!test
+%! if exist (testfile1, 'file')
+%!   delete (testfile1);
+%! endif
+%! if exist (testfile2, 'file')
+%!   delete (testfile2);
+%! endif
+*/
diff --git a/src/dicomdict.cpp b/src/dicomdict.cpp
index 1c2c19e..1a8e7ef 100644
--- a/src/dicomdict.cpp
+++ b/src/dicomdict.cpp
@@ -9,7 +9,7 @@
  * Minor changes Copyright Kris Thielemans 2011:
  *  make dicomdict('get') and dicomdict('set',filename) work properly and add doc-string
  *
- * Minor changes Copyright John Donoghue 2018-2019:
+ * Minor changes Copyright John Donoghue 2018-2020:
  * 
  * The GNU Octave dicom package is free software: you can redistribute 
  * it and/or modify it under the terms of the GNU General Public 
@@ -55,15 +55,15 @@ std::map<gdcm::Tag, std::string> tagmap ;
 std::map<std::string, gdcm::Tag> keymap ;
 std::map<std::string, gdcm::DictEntry> dict ;
 
-void insert(const char *k, const gdcm::Tag t, const gdcm::DictEntry e) {
-	keymap[k] = t ;
-	tagmap[t] = k ;
-	dict[k] = e ;
+void insert(const char *k, const gdcm::Tag t, const gdcm::DictEntry e)
+{
+  keymap[k] = t ;
+  tagmap[t] = k ;
+  dict[k] = e ;
 }
 
-
 DEFUN_DLD (dicomdict, args, nargout,
-		"-*- texinfo -*- \n\
+    "-*- texinfo -*- \n\
 @deftypefn {Loadable Function} {@var{dictionary_name} =} dicomdict (@code{get}) \n\
 @deftypefnx {Loadable Function} {} dicomdict (@code{factory}) \n\
 @deftypefnx {Loadable Function} {} dicomdict (@code{set}, @var{dictionary_name}) \n\
@@ -78,96 +78,110 @@ In this case, the dictionary file @var{dictionary_name} can be anywhere in the p
 @seealso{dicomread, dicomwrite}\n\
 @end deftypefn \n")
 {
-	octave_value_list retval;  // create object to store return values
-	if (args.length()>2 || args.length()<1) {
-		error(QUOTED(OCT_FN_NAME)": takes 1 or 2 arguments, got %i.", (int)args.length ());
-		return retval; 
-	}
-	if (! args(0).is_string ()) {
-		error(QUOTED(OCT_FN_NAME)": requires string as first argument");
-		return retval; 
-	}
-	charMatrix arg0mat = args(0).char_matrix_value ();
-	if (arg0mat.rows()!=1) {
-		error(QUOTED(OCT_FN_NAME)": first arg should be a single row string of chars.");
-		return retval; 
-	}
-	std::string arg0str = arg0mat.row_as_string (0);
-	if (args.length()==1) {
-		if (arg0str == std::string("get")) { // TODO: consider making args not case-sensitive
-			retval(0)=octave_value(dic_filename);
-			return retval;
-		} else if (arg0str == std::string("factory")) {
-			dic_filename=std::string(factory_dicom_dict_filename);
-// 			if (octave_dicom_dict == NULL) {
-// 				octave_dicom_dict = new OctaveDicomDict(); //TODO where should this be freed?
-// 				printf("init OctaveDicomDict before loading factory\n");
-// 			}
-			load_dicom_dict(dic_filename.c_str());
-			
-			return retval;
-		} else {
-			error(QUOTED(OCT_FN_NAME)": single arg must either be 'get' or 'factory'.");
-			return retval; 
-		}
-	}
-	//must be 2 args
-	charMatrix arg1mat = args(1).char_matrix_value ();
-	if (arg1mat.rows()!=1) {
-		error(QUOTED(OCT_FN_NAME)": second arg should be a single row string of chars.");
-		return retval; 
-	}
-	std::string arg1str = arg1mat.row_as_string (0);
-	if (arg0str != std::string("set")) {
-		error(QUOTED(OCT_FN_NAME)": when 2 args are given, the first must be 'set'.");
-		return retval; 
-	}
-	load_dicom_dict(arg1str.c_str());
-	//if (octave_dicom_dict == NULL) octave_dicom_dict = new OctaveDicomDict(); //TODO where should this be freed?
-	//octave_dicom_dict.load_file(arg1str.c_str()); // second arg is filename
-	return retval;
+  octave_value_list retval;  // create object to store return values
+  if (args.length () > 2 || args.length () < 1)
+    {
+      error (QUOTED(OCT_FN_NAME)": takes 1 or 2 arguments, got %i.", (int)args.length ());
+      return retval; 
+    }
+  if (! args(0).is_string ())
+    {
+      error (QUOTED(OCT_FN_NAME)": requires string as first argument");
+      return retval; 
+    }
+  charMatrix arg0mat = args(0).char_matrix_value ();
+  if (arg0mat.rows()!=1)
+    {
+      error(QUOTED(OCT_FN_NAME)": first arg should be a single row string of chars.");
+      return retval; 
+    }
+  std::string arg0str = arg0mat.row_as_string (0);
+  if (args.length () == 1)
+    {
+      if (arg0str == std::string ("get"))
+        { 
+          // TODO: consider making args not case-sensitive
+          retval (0) = octave_value (dic_filename);
+          return retval;
+        } 
+      else if (arg0str == std::string ("factory"))
+        {
+          dic_filename = std::string (factory_dicom_dict_filename);
+//        if (octave_dicom_dict == NULL) 
+//          {
+//            octave_dicom_dict = new OctaveDicomDict (); //TODO where should this be freed?
+//            printf ("init OctaveDicomDict before loading factory\n");
+//          }
+          load_dicom_dict (dic_filename.c_str());
+      
+          return retval;
+        }
+      else
+        {
+          error (QUOTED(OCT_FN_NAME)": single arg must either be 'get' or 'factory'.");
+          return retval; 
+        }
+    }
+  //must be 2 args
+  charMatrix arg1mat = args (1).char_matrix_value ();
+  if (arg1mat.rows () != 1)
+    {
+      error (QUOTED(OCT_FN_NAME)": second arg should be a single row string of chars.");
+      return retval; 
+    }
+  std::string arg1str = arg1mat.row_as_string (0);
+  if (arg0str != std::string ("set"))
+    {
+      error( QUOTED(OCT_FN_NAME)": when 2 args are given, the first must be 'set'.");
+      return retval; 
+    }
+  load_dicom_dict (arg1str.c_str());
+  //if (octave_dicom_dict == NULL) octave_dicom_dict = new OctaveDicomDict(); //TODO where should this be freed?
+  //octave_dicom_dict.load_file(arg1str.c_str()); // second arg is filename
+  return retval;
 }
 
 // Map from VR strings to gdcm Value Representations.
 typedef std::map<std::string, gdcm::VR> vr_map ;
-const vr_map::value_type vrData[] = {
-  vr_map::value_type("AE", gdcm::VR::AE),
-  vr_map::value_type("AS", gdcm::VR::AS),
-  vr_map::value_type("AT", gdcm::VR::AT),
-  vr_map::value_type("CS", gdcm::VR::CS),
-  vr_map::value_type("DA", gdcm::VR::DA),
-  vr_map::value_type("DS", gdcm::VR::DS),
-  vr_map::value_type("DT", gdcm::VR::DT),
-  vr_map::value_type("FD", gdcm::VR::FD),
-  vr_map::value_type("FL", gdcm::VR::FL),
-  vr_map::value_type("IS", gdcm::VR::IS),
-  vr_map::value_type("LO", gdcm::VR::LO),
-  vr_map::value_type("LT", gdcm::VR::LT),
-  vr_map::value_type("OB", gdcm::VR::OB),
-  vr_map::value_type("OF", gdcm::VR::OF),
-  vr_map::value_type("OW", gdcm::VR::OW),
-  vr_map::value_type("PN", gdcm::VR::PN),
-  vr_map::value_type("SH", gdcm::VR::SH),
-  vr_map::value_type("SL", gdcm::VR::SL),
-  vr_map::value_type("SQ", gdcm::VR::SQ),
-  vr_map::value_type("SS", gdcm::VR::SS),
-  vr_map::value_type("ST", gdcm::VR::ST),
-  vr_map::value_type("TM", gdcm::VR::TM),
-  vr_map::value_type("UI", gdcm::VR::UI),
-  vr_map::value_type("UL", gdcm::VR::UL),
-  vr_map::value_type("UN", gdcm::VR::UN),
-  vr_map::value_type("US", gdcm::VR::US),
-  vr_map::value_type("UT", gdcm::VR::UT),
-  vr_map::value_type("OB/OW", gdcm::VR::OB_OW),
-  vr_map::value_type("OW/OB", gdcm::VR::OB_OW),
-  vr_map::value_type("US/SS", gdcm::VR::US_SS),
-  vr_map::value_type("SS/US", gdcm::VR::US_SS),
-  vr_map::value_type("OW/SS/US", gdcm::VR::US_SS_OW),
-  vr_map::value_type("OW/US/SS", gdcm::VR::US_SS_OW),
-  vr_map::value_type("SS/OW/US", gdcm::VR::US_SS_OW),
-  vr_map::value_type("SS/US/OW", gdcm::VR::US_SS_OW),
-  vr_map::value_type("US/OW/SS", gdcm::VR::US_SS_OW),
-  vr_map::value_type("US/SS/OW", gdcm::VR::US_SS_OW)
+const vr_map::value_type vrData[] =
+{
+  vr_map::value_type ("AE", gdcm::VR::AE),
+  vr_map::value_type ("AS", gdcm::VR::AS),
+  vr_map::value_type ("AT", gdcm::VR::AT),
+  vr_map::value_type ("CS", gdcm::VR::CS),
+  vr_map::value_type ("DA", gdcm::VR::DA),
+  vr_map::value_type ("DS", gdcm::VR::DS),
+  vr_map::value_type ("DT", gdcm::VR::DT),
+  vr_map::value_type ("FD", gdcm::VR::FD),
+  vr_map::value_type ("FL", gdcm::VR::FL),
+  vr_map::value_type ("IS", gdcm::VR::IS),
+  vr_map::value_type ("LO", gdcm::VR::LO),
+  vr_map::value_type ("LT", gdcm::VR::LT),
+  vr_map::value_type ("OB", gdcm::VR::OB),
+  vr_map::value_type ("OF", gdcm::VR::OF),
+  vr_map::value_type ("OW", gdcm::VR::OW),
+  vr_map::value_type ("PN", gdcm::VR::PN),
+  vr_map::value_type ("SH", gdcm::VR::SH),
+  vr_map::value_type ("SL", gdcm::VR::SL),
+  vr_map::value_type ("SQ", gdcm::VR::SQ),
+  vr_map::value_type ("SS", gdcm::VR::SS),
+  vr_map::value_type ("ST", gdcm::VR::ST),
+  vr_map::value_type ("TM", gdcm::VR::TM),
+  vr_map::value_type ("UI", gdcm::VR::UI),
+  vr_map::value_type ("UL", gdcm::VR::UL),
+  vr_map::value_type ("UN", gdcm::VR::UN),
+  vr_map::value_type ("US", gdcm::VR::US),
+  vr_map::value_type ("UT", gdcm::VR::UT),
+  vr_map::value_type ("OB/OW", gdcm::VR::OB_OW),
+  vr_map::value_type ("OW/OB", gdcm::VR::OB_OW),
+  vr_map::value_type ("US/SS", gdcm::VR::US_SS),
+  vr_map::value_type ("SS/US", gdcm::VR::US_SS),
+  vr_map::value_type ("OW/SS/US", gdcm::VR::US_SS_OW),
+  vr_map::value_type ("OW/US/SS", gdcm::VR::US_SS_OW),
+  vr_map::value_type ("SS/OW/US", gdcm::VR::US_SS_OW),
+  vr_map::value_type ("SS/US/OW", gdcm::VR::US_SS_OW),
+  vr_map::value_type ("US/OW/SS", gdcm::VR::US_SS_OW),
+  vr_map::value_type ("US/SS/OW", gdcm::VR::US_SS_OW)
 };
 const int vrDataLength = sizeof vrData / sizeof vrData[0];
 static vr_map vrMap(vrData, vrData+vrDataLength) ;
@@ -175,44 +189,45 @@ static vr_map vrMap(vrData, vrData+vrDataLength) ;
 
 // Map from VM strings to gdcm Value Multipicities.
 typedef std::map<std::string, gdcm::VM> vm_map ;
-const vm_map::value_type vmData[] = {
-  vm_map::value_type("0", gdcm::VM::VM0),
-  vm_map::value_type("1", gdcm::VM::VM1),
-  vm_map::value_type("2", gdcm::VM::VM2),
-  vm_map::value_type("3", gdcm::VM::VM3),
-  vm_map::value_type("4", gdcm::VM::VM4),
-  vm_map::value_type("5", gdcm::VM::VM5),
-  vm_map::value_type("6", gdcm::VM::VM6),
-  vm_map::value_type("8", gdcm::VM::VM8),
-  vm_map::value_type("9", gdcm::VM::VM9),
-  vm_map::value_type("10", gdcm::VM::VM10),
-  vm_map::value_type("12", gdcm::VM::VM12),
-  vm_map::value_type("16", gdcm::VM::VM16),
-  vm_map::value_type("18", gdcm::VM::VM18),
-  vm_map::value_type("24", gdcm::VM::VM24),
-  vm_map::value_type("28", gdcm::VM::VM28),
-  vm_map::value_type("32", gdcm::VM::VM32),
-  vm_map::value_type("35", gdcm::VM::VM35),
-  vm_map::value_type("99", gdcm::VM::VM99),
-  vm_map::value_type("256", gdcm::VM::VM256),
-  vm_map::value_type("1-2", gdcm::VM::VM1_2),
-  vm_map::value_type("1-3", gdcm::VM::VM1_3),
-  vm_map::value_type("1-4", gdcm::VM::VM1_4),
-  vm_map::value_type("1-5", gdcm::VM::VM1_5),
-  vm_map::value_type("1-8", gdcm::VM::VM1_8),
-  vm_map::value_type("1-32", gdcm::VM::VM1_32),
-  vm_map::value_type("1-99", gdcm::VM::VM1_99),
-  vm_map::value_type("1-n", gdcm::VM::VM1_n),
-  vm_map::value_type("2-2n", gdcm::VM::VM2_2n),
-  vm_map::value_type("2-n", gdcm::VM::VM2_n),
-  vm_map::value_type("3-4", gdcm::VM::VM3_4),
-  vm_map::value_type("3-3n", gdcm::VM::VM3_3n),
-  vm_map::value_type("3-n", gdcm::VM::VM3_n),
-  vm_map::value_type("4-4n", gdcm::VM::VM4_4n),
-  vm_map::value_type("6-6n", gdcm::VM::VM6_6n),
-  vm_map::value_type("7-7n", gdcm::VM::VM7_7n),
-  vm_map::value_type("30-30n", gdcm::VM::VM30_30n),
-  vm_map::value_type("47-47n", gdcm::VM::VM47_47n)
+const vm_map::value_type vmData[] =
+{
+  vm_map::value_type ("0", gdcm::VM::VM0),
+  vm_map::value_type ("1", gdcm::VM::VM1),
+  vm_map::value_type ("2", gdcm::VM::VM2),
+  vm_map::value_type ("3", gdcm::VM::VM3),
+  vm_map::value_type ("4", gdcm::VM::VM4),
+  vm_map::value_type ("5", gdcm::VM::VM5),
+  vm_map::value_type ("6", gdcm::VM::VM6),
+  vm_map::value_type ("8", gdcm::VM::VM8),
+  vm_map::value_type ("9", gdcm::VM::VM9),
+  vm_map::value_type ("10", gdcm::VM::VM10),
+  vm_map::value_type ("12", gdcm::VM::VM12),
+  vm_map::value_type ("16", gdcm::VM::VM16),
+  vm_map::value_type ("18", gdcm::VM::VM18),
+  vm_map::value_type ("24", gdcm::VM::VM24),
+  vm_map::value_type ("28", gdcm::VM::VM28),
+  vm_map::value_type ("32", gdcm::VM::VM32),
+  vm_map::value_type ("35", gdcm::VM::VM35),
+  vm_map::value_type ("99", gdcm::VM::VM99),
+  vm_map::value_type ("256", gdcm::VM::VM256),
+  vm_map::value_type ("1-2", gdcm::VM::VM1_2),
+  vm_map::value_type ("1-3", gdcm::VM::VM1_3),
+  vm_map::value_type ("1-4", gdcm::VM::VM1_4),
+  vm_map::value_type ("1-5", gdcm::VM::VM1_5),
+  vm_map::value_type ("1-8", gdcm::VM::VM1_8),
+  vm_map::value_type ("1-32", gdcm::VM::VM1_32),
+  vm_map::value_type ("1-99", gdcm::VM::VM1_99),
+  vm_map::value_type ("1-n", gdcm::VM::VM1_n),
+  vm_map::value_type ("2-2n", gdcm::VM::VM2_2n),
+  vm_map::value_type ("2-n", gdcm::VM::VM2_n),
+  vm_map::value_type ("3-4", gdcm::VM::VM3_4),
+  vm_map::value_type ("3-3n", gdcm::VM::VM3_3n),
+  vm_map::value_type ("3-n", gdcm::VM::VM3_n),
+  vm_map::value_type ("4-4n", gdcm::VM::VM4_4n),
+  vm_map::value_type ("6-6n", gdcm::VM::VM6_6n),
+  vm_map::value_type ("7-7n", gdcm::VM::VM7_7n),
+  vm_map::value_type ("30-30n", gdcm::VM::VM30_30n),
+  vm_map::value_type ("47-47n", gdcm::VM::VM47_47n)
 };
 const int vmDataLength = sizeof vmData / sizeof vmData[0];
 static vm_map vmMap(vmData, vmData+vmDataLength) ;
@@ -220,165 +235,191 @@ static vm_map vmMap(vmData, vmData+vmDataLength) ;
 
 // A simple iterator for tag ranges
 // (e.g. "12xx" goes from 0x1200 to 0x12ff)
-class tag_range_iter {
+class tag_range_iter
+{
 private:
   uint16_t val ;
   uint16_t mask ;
 
 public:
-  tag_range_iter(const std::string tag) : val(0x0000), mask(0x0000)
+  tag_range_iter (const std::string tag) : val(0x0000), mask(0x0000)
   {
     char tmp[] = "...." ;
-    for ( size_t i = 0 ; i<4 ; i++ )
+    for (size_t i = 0; i < 4; i++)
       {
-	mask<<=4;
-	if ( tag[i] == 'x' || tag[i] == 'X' )
-	  {
-	    tmp[i] = '0' ;
-	    mask |= 0x000f ;
-	  }
-	else
-	  {
-	    tmp[i] = tag[i] ;
-	  }
+        mask<<=4;
+        if (tag[i] == 'x' || tag[i] == 'X')
+          {
+            tmp[i] = '0';
+            mask |= 0x000f;
+          }
+        else
+          {
+            tmp[i] = tag[i];
+          }
       }
     unsigned int v ;
-    sscanf(tmp, "%4x", &v) ;
+    sscanf(tmp, "%4x", &v);
     val = v ;
   }
 
-  bool operator++()
+  bool operator++ ()
   {
-    if (val==(val|mask))
+    if (val == (val|mask))
       {
-	return false ;
+        return false;
       }
-    val = (~mask&val)|((((mask&val)|(~mask))+1)&mask) ;
-    return true ;
+    val = (~mask&val)|((((mask&val)|(~mask))+1)&mask);
+    return true;
   }
   
-  const uint16_t value() {
-    return val ;
+  const uint16_t value ()
+  {
+    return val;
   }
 };
 
-const char * const get_current_dicom_dict() {
+const char * const get_current_dicom_dict ()
+{
   return dic_filename.c_str();
 }
 
-void load_dicom_dict(const char * filename) {
-	// reset, if required
-	if (tagmap.size()>0) {
-		tagmap.clear() ;
-		keymap.clear() ;
-		dict.clear() ;
-	}
-
-	// find dic if it is anywhere in the search path (same path as for m-files etc)
-	std::string resolved_filename(filename);
+void load_dicom_dict (const char * filename)
+{
+  // reset, if required
+  if (tagmap.size()>0)
+    {
+      tagmap.clear () ;
+      keymap.clear () ;
+      dict.clear () ;
+    }
+
+  // find dic if it is anywhere in the search path (same path as for m-files etc)
+  std::string resolved_filename(filename);
 #ifndef NOT_OCT
 #if HAVE_OCTAVE_LOAD_PATH == 1
-	octave::interpreter *interp = octave::interpreter::the_interpreter ();
-	if (interp) {
-		octave::load_path& lp = interp->get_load_path ();
-		resolved_filename = lp.find_file (std::string (filename));
-	}
-	else
-		warning ("load_dicom_dict: interpreter context missing");
+  octave::interpreter *interp = octave::interpreter::the_interpreter ();
+  if (interp)
+    {
+      octave::load_path& lp = interp->get_load_path ();
+      resolved_filename = lp.find_file (std::string (filename));
+    }
+  else
+    warning ("load_dicom_dict: interpreter context missing");
 #else
-	resolved_filename=load_path::find_file(std::string(filename));
+  resolved_filename=load_path::find_file(std::string(filename));
 #endif // HAVE_OCTAVE_LOAD_PATH
 #endif // NOT_OCT
 
-	std::ifstream fin(resolved_filename.c_str());
-	if (!fin) {
-		error( "Failed to open dic" ) ;
-		return ;
-	}
-
-	// Process each line
-	size_t linenumber = 0 ;
-	while (!fin.eof()) {
-		std::string line ;
-		getline(fin,line) ;
-		linenumber++ ;
-
-		// Skip any line that start with "#" without complaining
-		if ( line[0] == '#' ) continue ;
-
-		// Skip lines that don't start with "(xxxx,xxxx)"
-		if ( (line.size() < 11)
-			|| (line[0] != '(')
-			|| (line[5] != ',')
-			|| (line[10] != ')') )	{
-			continue ;
-		}
-
-		char tgroup[4+1] ;
-		char telem[4+1] ;
-		char tvr[8+1] ;
-		char key[128+1] ;
-		char tvm[8+1] ;
-
-		// Tokenize line
-		if ( sscanf(line.c_str(), "(%4s,%4s) %8s %128s %8s", tgroup, telem, tvr, key, tvm ) != 5 ) {
-			continue ;
-		}
-
-		// Convert VR
-		gdcm::VR vr = vrMap[tvr] ;
-
-		// Convert VM
-		gdcm::VM vm = vmMap[tvm] ;
-
-		// Warn if keyword cannot be used in octave
-		if ( ! OCTAVE__VALID_IDENTIFIER (key) ) {
-			std::cerr << "WARNING: Invalid identifier '" << key << "'" << std::endl << std::flush ;
-		}
-
-		gdcm::DictEntry entry ;      
-		entry.SetVR (vr) ;
-		entry.SetVM (vm) ;
-		entry.SetName (key) ;
-
-		gdcm::Tag tag ;
-		tag_range_iter group(tgroup) ;      
-		do	{
-			tag.SetGroup(group.value()) ;
-			tag_range_iter elem(telem) ;
-			do	{
-				tag.SetElement(elem.value()) ;
-				insert(key,tag,entry) ;
-			} while ( ++elem ) ;
-		} while ( ++group ) ;
-	}
-	// save filename
-	dic_filename = resolved_filename;
+  std::ifstream fin (resolved_filename.c_str ());
+  if (!fin)
+    {
+      error( "Failed to open dic" );
+      return;
+    }
+
+  // Process each line
+  size_t linenumber = 0;
+  while (!fin.eof())
+    {
+      std::string line ;
+      getline(fin,line) ;
+      linenumber++ ;
+
+      // Skip any line that start with "#" without complaining
+      if (line[0] == '#') continue;
+
+      // Skip lines that don't start with "(xxxx,xxxx)"
+      if ( (line.size() < 11)
+        || (line[0] != '(')
+        || (line[5] != ',')
+        || (line[10] != ')') )
+        {
+          continue ;
+        }
+
+      char tgroup[4+1] ;
+      char telem[4+1] ;
+      char tvr[8+1] ;
+      char key[128+1] ;
+      char tvm[8+1] ;
+
+      // Tokenize line
+      if (sscanf(line.c_str(), "(%4s,%4s) %8s %128s %8s", tgroup, telem, tvr, key, tvm ) != 5)
+        {
+          continue;
+        }
+
+      // Convert VR
+      gdcm::VR vr = vrMap[tvr];
+
+      // Convert VM
+      gdcm::VM vm = vmMap[tvm];
+
+      // Warn if keyword cannot be used in octave
+      if (! OCTAVE__VALID_IDENTIFIER (key))
+        {
+          std::cerr << "WARNING: Invalid identifier '" << key << "'" << std::endl << std::flush;
+       }
+
+      gdcm::DictEntry entry;
+      entry.SetVR (vr);
+      entry.SetVM (vm);
+      entry.SetName (key);
+
+      gdcm::Tag tag ;
+      tag_range_iter group (tgroup);      
+      do 
+        {
+          tag.SetGroup (group.value ());
+          tag_range_iter elem(telem);
+          do
+            {
+              tag.SetElement(elem.value());
+              insert(key,tag,entry);
+            }
+          while (++elem);
+        }
+      while ( ++group );
+    }
+
+  // save filename
+  dic_filename = resolved_filename;
 }
 
-void lookup_dicom_keyword(std::string & keyword, const gdcm::Tag & tag) {
-	if (0==tagmap.size()) load_dicom_dict(factory_dicom_dict_filename); // init if necessary
-	keyword = tagmap[tag];
+void lookup_dicom_keyword (std::string & keyword, const gdcm::Tag & tag)
+{
+  if (0 == tagmap.size ())
+    load_dicom_dict (factory_dicom_dict_filename); // init if necessary
+  keyword = tagmap[tag];
 }
 
-void lookup_dicom_tag(gdcm::Tag & tag, const std::string & keyword) {
-	if (0==tagmap.size()) load_dicom_dict(factory_dicom_dict_filename); // init if necessary
-	tag = gdcm::Tag(keymap[keyword]);
+void lookup_dicom_tag (gdcm::Tag & tag, const std::string & keyword)
+{
+  if (0 == tagmap.size())
+    load_dicom_dict (factory_dicom_dict_filename); // init if necessary
+  tag = gdcm::Tag (keymap[keyword]);
 }
 
-void lookup_dicom_entry(gdcm::DictEntry & entry, const gdcm::Tag & tag) {
-	if (0==tagmap.size()) load_dicom_dict(factory_dicom_dict_filename); // init if necessary
-	entry = gdcm::DictEntry(dict[tagmap[tag]]);
+void lookup_dicom_entry (gdcm::DictEntry & entry, const gdcm::Tag & tag)
+{
+  if (0 == tagmap.size())
+    load_dicom_dict (factory_dicom_dict_filename); // init if necessary
+  entry = gdcm::DictEntry (dict[tagmap[tag]]);
 }
 
-bool dicom_is_present(const std::string & keyword){
-	if (0==tagmap.size()) load_dicom_dict(factory_dicom_dict_filename); // init if necessary
-	return keymap.count(keyword)>(std::vector<std::string>::size_type)0 ;
+bool dicom_is_present (const std::string & keyword)
+{
+  if (0 == tagmap.size ())
+    load_dicom_dict (factory_dicom_dict_filename); // init if necessary
+  return keymap.count(keyword) > (std::vector<std::string>::size_type)0;
 }
 
-bool dicom_is_present(const gdcm::Tag & tag){
-	if (0==tagmap.size()) load_dicom_dict(factory_dicom_dict_filename); // init if necessary
-	return tagmap.count(tag)>(std::vector<gdcm::Tag>::size_type)0 ;
+bool dicom_is_present (const gdcm::Tag & tag)
+{
+  if (0 == tagmap.size ())
+    load_dicom_dict (factory_dicom_dict_filename); // init if necessary
+  return tagmap.count(tag) > (std::vector<gdcm::Tag>::size_type)0;
 }
 
 /*
diff --git a/src/dicomdict.h b/src/dicomdict.h
index d5419f2..ee6a47a 100644
--- a/src/dicomdict.h
+++ b/src/dicomdict.h
@@ -30,6 +30,6 @@ bool dicom_is_present(const gdcm::Tag & tag);
   * contrast with some VRASCII types that hold numbers.
   * may take some dates and times out of this and handle differently */
 #define VRSTRING (gdcm::VR::AE|gdcm::VR::AS|gdcm::VR::CS|gdcm::VR::DA\
-	|gdcm::VR::DT|gdcm::VR::LO|gdcm::VR::LT|gdcm::VR::PN|gdcm::VR::SH\
-	|gdcm::VR::ST|gdcm::VR::TM|gdcm::VR::UI|gdcm::VR::UT)
+  |gdcm::VR::DT|gdcm::VR::LO|gdcm::VR::LT|gdcm::VR::PN|gdcm::VR::SH\
+  |gdcm::VR::ST|gdcm::VR::TM|gdcm::VR::UI|gdcm::VR::UT)
 
diff --git a/src/dicomdisp.cpp b/src/dicomdisp.cpp
index f595f50..c7a03f7 100644
--- a/src/dicomdisp.cpp
+++ b/src/dicomdisp.cpp
@@ -301,7 +301,8 @@ void dumpElement(const gdcm::DataElement * elem, int sequenceDepth, uint64_t &of
   if (vr & gdcm::VR::SQ)
     {
       gdcm::SmartPointer<gdcm::SequenceOfItems> sqi = elem->GetValueAsSQ();
-      dumpSequence(sqi, sequenceDepth+1, offset);
+      if (sqi)
+        dumpSequence(sqi, sequenceDepth+1, offset);
     }
 }
 
diff --git a/src/dicominfo.cpp b/src/dicominfo.cpp
index 2d4c43d..a4b98cc 100644
--- a/src/dicominfo.cpp
+++ b/src/dicominfo.cpp
@@ -1,5 +1,5 @@
 /*
- * The GNU Octave dicom package is Copyright Andy Buckle 2010
+ * The GNU Octave dicom package is Copyright Andy Buckle 2010-2020
  * Contact: blondandy using the sf.net system, 
  * <https://sourceforge.net/sendmessage.php?touser=1760416>
  * 
@@ -53,8 +53,6 @@
 
 #include "dicomdict.h" 
 
-
-  
 #define DICOM_ERR -1
 #define DICOM_OK 0
 #define DICOM_NOTHING_ASSIGNED 1
@@ -66,160 +64,186 @@
 #define QUOTED(x) QUOTED_(x)
 
 #ifdef NOT_OCT
-#	define octave_stdout	std::cout
-#	define error			printf
+# define octave_stdout  std::cout
+# define error      printf
 #endif
 
-char* byteval2string(char * d, int d_len_p, const gdcm::ByteValue *bv);
-char* name2Keyword(char *d, int *d_len_p, const char* s);
-Matrix str2DoubleVec(const char*);
-octave_map dump(const char filename[], int chatty);
-void dumpDataSet(octave_map *om, const gdcm::DataSet *ds, int chatty, int sequenceDepth);
-void getFileModTime(char *timeStr, const char *filename);
-void dumpElement(octave_map *om, const gdcm::DataElement * elem, int chatty, int sequenceDepth);
-void dumpSequence(octave_value *ov, gdcm::SequenceOfItems *seq, int chatty, int sequenceDepth);
-int element2value(std::string & varname, octave_value *ov, const gdcm::DataElement * elem, int chatty, int sequenceDepth) ;
+char* byteval2string (char * d, int d_len_p, const gdcm::ByteValue *bv);
+char* name2Keyword (char *d, int *d_len_p, const char* s);
+Matrix str2DoubleVec (const char*);
+octave_map dump (const char filename[], int chatty);
+void dumpDataSet (octave_map *om, const gdcm::DataSet *ds, int chatty, int sequenceDepth);
+void getFileModTime (char *timeStr, const char *filename);
+void dumpElement (octave_map *om, const gdcm::DataElement * elem, int chatty, int sequenceDepth);
+void dumpSequence (octave_value *ov, gdcm::SequenceOfItems *seq, int chatty, int sequenceDepth);
+int element2value (std::string & varname, octave_value *ov, const gdcm::DataElement * elem, int chatty, int sequenceDepth) ;
 
 int dicom_truncate_numchar=40;
 
 #ifdef NOT_OCT
-int main( int argc, const char* argv[] ) {
-	dump(argv[1], 1 /* chatty on */ ); // 1 cmd line arg: dicom filename
-	return 0;
+int main(int argc, const char* argv[])
+{
+  dump (argv[1], 1 /* chatty on */ ); // 1 cmd line arg: dicom filename
+  return 0;
 }
 #else
 DEFUN_DLD (dicominfo, args, nargout,
-		"-*- texinfo -*- \n\
- @deftypefn {Loadable Function} {@var{info}} = dicominfo (@var{filename}) \n\
- @deftypefnx {Loadable Function} {@var{info}} = dicominfo (@var{filename}, @code{dictionary}, @var{dictionary-name}) \n\
- @deftypefnx  {Loadable Function} {} dicominfo (@var{filename}, @var{options}) \n\
- @deftypefnx {Command} {} dicominfo @var{filename} \n\
- @deftypefnx {Command} {} dicominfo @var{filename} @var{options} \n\
- Get all data from a DICOM file, excluding any actual image. \n\
- @var{info} is a nested struct containing the data. \n\
- \n\
- If no return argument is given, then there will be output similar to \n\
- a DICOM dump. \n\
- \n\
- If the @code{dictionary} argument is used, the given @var{dictionary-name} is used for this operation, \n\
- otherwise, the dictionary set by @code{dicomdict} is used.\n\
- \n\
- @var{options}:\n\
- @code{truncate=n}\n\
- where n is the number of characters to limit the dump output display to @code{n}\
- for each value. \n\
+    "-*- texinfo -*- \n\
+@deftypefn {Loadable Function} {@var{info}} = dicominfo (@var{filename}) \n\
+@deftypefnx {Loadable Function} {@var{info}} = dicominfo (@var{filename}, @code{dictionary}, @var{dictionary-name}) \n\
+@deftypefnx  {Loadable Function} {} dicominfo (@var{filename}, @var{options}) \n\
+@deftypefnx {Command} {} dicominfo @var{filename} \n\
+@deftypefnx {Command} {} dicominfo @var{filename} @var{options} \n\
+Get all data from a DICOM file, excluding any actual image. \n\
+@var{info} is a nested struct containing the data. \n\
+\n\
+If no return argument is given, then there will be output similar to \n\
+a DICOM dump. \n\
 \n\
- @seealso{dicomread, dicomdict} \n\
- @end deftypefn\n\
- ")
+If the @code{dictionary} argument is used, the given @var{dictionary-name} is used for this operation, \n\
+otherwise, the dictionary set by @code{dicomdict} is used.\n\
+\n\
+@var{options}:\n\
+@code{truncate=n}\n\
+where n is the number of characters to limit the dump output display to @code{n}\
+for each value. \n\
+\n\
+@seealso{dicomread, dicomdict} \n\
+@end deftypefn\n\
+")
 {
-	octave_value_list retval;  // create object to store return values
-	if ( (0 == args.length()) || (! args(0).is_string ())) {
-		error(QUOTED(OCT_FN_NAME)": one arg required: dicom filename");
-		return retval; 
-	}
-	int chatty = !nargout; // dump output to stdout if not assigning to var
-	charMatrix ch = args(0).char_matrix_value ();
-	if (ch.rows()!=1) {
-		error(QUOTED(OCT_FN_NAME)": arg should be a filename, 1 row of chars");
-		return retval; 
-	}
-	std::string filename = ch.row_as_string (0);
-
-	// save current dictionary for the case that we're using a different dictionary while reading
-	const std::string current_dict = get_current_dicom_dict();
-
-	int i; // parse any additional args
-	for (i=1; i<args.length(); i++) {
-		charMatrix chex = args(i).char_matrix_value();
-		if (chex.rows()!=1) {
-			error(QUOTED(OCT_FN_NAME)": arg should be a string, 1 row of chars");
-			load_dicom_dict(current_dict.c_str()); // reset dictionary to initial value
-			return retval; 
-		}
-		std::string argex = chex.row_as_string (0);
-		if (!argex.compare("dictionary") || !argex.compare("Dictionary")) {
-		        if (i+1==args.length()) {
-			      error(QUOTED(OCT_FN_NAME)": Dictionary needs another argument");
-			      load_dicom_dict(current_dict.c_str()); // reset dictionary to initial value
-			      return retval;
-			}
-			if (!args(i+1).is_string()) {
-			      error(QUOTED(OCT_FN_NAME)": Dictionary needs a string argument");
-			      load_dicom_dict(current_dict.c_str()); // reset dictionary to initial value
-			      return retval;
-			}
-			std::string dictionary = args(i+1).string_value();
-			load_dicom_dict(dictionary.c_str());
-			// ignore dictionary argument for further arg processing
-			++i;
-		}
-		else if (!argex.compare(0,9,"truncate=")) {
-			dicom_truncate_numchar=atoi(argex.substr(9).c_str());
-		} else {
-			warning(QUOTED(OCT_FN_NAME)": arg not understood: %s", argex.c_str());
-		}
-	}
-	
-	octave_map om=dump(filename.c_str(),chatty);
-	retval(0)=om;
-
-	load_dicom_dict(current_dict.c_str()); // reset dictionary to initial value
-	return retval;
+  octave_value_list retval;  // create object to store return values
+  if ( (0 == args.length ()) || (! args (0).is_string ()))
+    {
+      error(QUOTED(OCT_FN_NAME)": one arg required: dicom filename");
+      return retval; 
+    }
+  int chatty = !nargout; // dump output to stdout if not assigning to var
+  charMatrix ch = args(0).char_matrix_value ();
+  if (ch.rows () != 1)
+    {
+      error (QUOTED(OCT_FN_NAME)": arg should be a filename, 1 row of chars");
+      return retval; 
+    }
+  std::string filename = ch.row_as_string (0);
+
+  // save current dictionary for the case that we're using a different dictionary while reading
+  const std::string current_dict = get_current_dicom_dict ();
+
+  int i; // parse any additional args
+  for (i = 1; i < args.length(); i++)
+    {
+      charMatrix chex = args(i).char_matrix_value();
+      if (chex.rows () != 1)
+        {
+          error (QUOTED(OCT_FN_NAME)": arg should be a string, 1 row of chars");
+          load_dicom_dict (current_dict.c_str ()); // reset dictionary to initial value
+         return retval; 
+        }
+      std::string argex = chex.row_as_string (0);
+      if (!argex.compare ("dictionary") || !argex.compare ("Dictionary"))
+        {
+          if (i+1 == args.length())
+            {
+              error (QUOTED(OCT_FN_NAME)": Dictionary needs another argument");
+              load_dicom_dict (current_dict.c_str ()); // reset dictionary to initial value
+              return retval;
+            }
+          if (!args (i+1).is_string ())
+            {
+              error (QUOTED(OCT_FN_NAME)": Dictionary needs a string argument");
+              load_dicom_dict (current_dict.c_str()); // reset dictionary to initial value
+              return retval;
+            }
+          std::string dictionary = args (i+1).string_value ();
+          load_dicom_dict (dictionary.c_str());
+          // ignore dictionary argument for further arg processing
+          ++i;
+        }
+      else if (!argex.compare (0,9,"truncate="))
+        {
+          dicom_truncate_numchar = atoi (argex.substr (9).c_str ());
+        }
+      else
+        {
+          warning (QUOTED(OCT_FN_NAME)": arg not understood: %s", argex.c_str());
+        }
+    }
+  
+  octave_map om = dump (filename.c_str (), chatty);
+  retval(0) = om;
+
+  load_dicom_dict (current_dict.c_str ()); // reset dictionary to initial value
+  return retval;
 }
 #endif
 
 
-octave_map dump(const char filename[], int chatty) {
-	// output struct
-	octave_map om;
-	// Instantiate the reader:
-	gdcm::Reader reader;
-	reader.SetFileName( filename );
-	if( !reader.Read() ) {
-		error("Could not read: %s",filename);
-		return om; //TODO: set error state somehow so the main DEFUN_DLD function knows
-		           // KT this doesn't seem to be necessary. Presumably error() sets a flag that tells the interpreter it should abort
-	}
-	gdcm::File &file = reader.GetFile();
-	gdcm::DataSet &ds = file.GetDataSet();
-	gdcm::FileMetaInformation &hds=file.GetHeader();
-	
-	om.assign("Filename", octave_value(filename));
-	char dateStr[TIME_STR_LEN+1];
-	getFileModTime(dateStr, filename);
-	om.assign("FileModDate", octave_value(dateStr));
-	if(chatty) octave_stdout << "# file info\nFilename:" 
-		<< filename << "\nFileModDate:" << dateStr << '\n';
-	
-	if(chatty) octave_stdout << "# header\n" ;
-	dumpDataSet(&om, &hds, chatty, 0);
-	if(chatty) octave_stdout << "# metadata\n" ;
-	dumpDataSet(&om, &ds, chatty, 0);
-	
-	return om;
+octave_map dump (const char filename[], int chatty)
+{
+  // output struct
+  octave_map om;
+  // Instantiate the reader:
+  gdcm::Reader reader;
+  reader.SetFileName (filename);
+  if (!reader.Read ())
+    {
+      error("Could not read: %s",filename);
+      //TODO: set error state somehow so the main DEFUN_DLD function knows
+      // KT this doesn't seem to be necessary. Presumably error() sets a flag that tells the interpreter it should abort
+      return om; 
+    }
+  gdcm::File &file = reader.GetFile ();
+  gdcm::DataSet &ds = file.GetDataSet ();
+  gdcm::FileMetaInformation &hds = file.GetHeader ();
+  
+  om.assign ("Filename", octave_value (filename));
+  char dateStr[TIME_STR_LEN+1];
+  getFileModTime (dateStr, filename);
+  om.assign ("FileModDate", octave_value (dateStr));
+  if(chatty)
+    {
+      octave_stdout << "# file info\nFilename:" 
+        << filename << "\nFileModDate:" << dateStr << '\n';
+      octave_stdout << "# header\n";
+    }
+  dumpDataSet (&om, &hds, chatty, 0);
+  if(chatty)
+    octave_stdout << "# metadata\n";
+
+  dumpDataSet(&om, &ds, chatty, 0);
+  
+  return om;
 }
 
-void dumpDataSet(octave_map *om, const gdcm::DataSet *ds, int chatty, int sequenceDepth) {
-	
-	const gdcm::DataSet::DataElementSet DES=ds->GetDES(); // gdcm::DataSet::DataElementSet is a std::set
-	gdcm::DataSet::Iterator it;
-	
-	for ( it=DES.begin() ; it != DES.end(); it++ ) {		
-		dumpElement(om, &(*it), chatty, sequenceDepth);
-	}
+void dumpDataSet(octave_map *om, const gdcm::DataSet *ds, int chatty, int sequenceDepth)
+{  
+  const gdcm::DataSet::DataElementSet DES = ds->GetDES (); // gdcm::DataSet::DataElementSet is a std::set
+  gdcm::DataSet::Iterator it;
+  if(ds)
+    {
+      for (it = DES.begin(); it != DES.end(); it++ )
+        {
+          dumpElement (om, &(*it), chatty, sequenceDepth);
+        }
+    }
 }
 
 void dumpElement(octave_map *om, const gdcm::DataElement * elem, 
-				int chatty, int sequenceDepth) {
-	std::string varname;
-	octave_value ov;
-	if(DICOM_OK==element2value(varname, &ov, elem, chatty, sequenceDepth)) {
-		om->assign(varname.c_str(), ov);
-	} else {
-		if (0==varname.length()) return ;
-		om->assign(varname.c_str(), octave_value("not assigned"));
-	}
+        int chatty, int sequenceDepth)
+{
+  std::string varname;
+  octave_value ov;
+
+  if (DICOM_OK == element2value(varname, &ov, elem, chatty, sequenceDepth))
+    {
+      om->assign (varname.c_str (), ov);
+    }
+  else
+    {
+      if (0 == varname.length()) return;
+      om->assign (varname.c_str(), octave_value("not assigned"));
+    }
 }
 
 /* helper function for element2value below.
@@ -241,43 +265,46 @@ void dumpElement(octave_map *om, const gdcm::DataElement * elem,
 template <gdcm::VR::VRType vrtype, typename valueType, typename octaveArrayType>
 static inline
 int element2simplevalueHelper2(octave_value *ov, const gdcm::DataElement *elem,
-			    const int chatty) {
-    if( elem->IsEmpty() )
-      return DICOM_NOTHING_ASSIGNED;
+          const int chatty)
+{
+  if (elem->IsEmpty())
+    return DICOM_NOTHING_ASSIGNED;
 
 #if 0 // KT if to choose between implementations
-    // original fast implementation using memcpy. However, ignores byteorder and VMs (i.e. arrays)
-    valueType val ; 
-    memcpy(&val, elem->GetByteValue()->GetPointer(), sizeof(valueType));
-    *ov=val;
-    if(chatty) octave_stdout << '[' << val << "]\n";
+  // original fast implementation using memcpy. However, ignores byteorder and VMs (i.e. arrays)
+  valueType val ; 
+  memcpy(&val, elem->GetByteValue()->GetPointer(), sizeof(valueType));
+  *ov=val;
+  if(chatty) octave_stdout << '[' << val << "]\n";
 #else
-    // save (but slow?) implementation that uses GDCM functions
-    gdcm::Element<vrtype,gdcm::VM::VM1_n> el;
-    el.Set( elem->GetValue() );
-    // possible optimisation here. If there's only 1 element, maybe we can
-    // save time by not making array. However, because all octave values are
-    // arrays, maybe this doesn't matter. the "else" statement below works
-    // always in any case.
-    // If you want to try this optimisation, put #if 1 below
-#   if 0
-    if (el.GetLength()==1)
-      {
-	const valueType& val = el.GetValue();
-	*ov=val;
-	if(chatty) octave_stdout << '[' << val << "]\n";
-      }
-    else
-#   endif
-      {
-	octaveArrayType val(dim_vector (el.GetLength(),1));
-	for (unsigned i=0; i<el.GetLength(); ++i)
-	  val(i)  = el.GetValue(i); 
-	*ov=val;
-	if(chatty) octave_stdout << '[' << val << "]\n";
-      }
+  // save (but slow?) implementation that uses GDCM functions
+  gdcm::Element<vrtype,gdcm::VM::VM1_n> el;
+  el.Set( elem->GetValue() );
+  // possible optimisation here. If there's only 1 element, maybe we can
+  // save time by not making array. However, because all octave values are
+  // arrays, maybe this doesn't matter. the "else" statement below works
+  // always in any case.
+  // If you want to try this optimisation, put #if 1 below
+# if 0
+  if (el.GetLength()==1)
+    {
+      const valueType& val = el.GetValue();
+      *ov=val;
+      if(chatty)
+        octave_stdout << '[' << val << "]\n";
+    }
+  else
+# endif
+  {
+    octaveArrayType val(dim_vector (el.GetLength(),1));
+    for (unsigned i = 0; i < el.GetLength(); ++i)
+    val(i)  = el.GetValue(i); 
+    *ov=val;
+    if (chatty)
+      octave_stdout << '[' << val << "]\n";
+  }
 #endif
-    return DICOM_OK;
+  return DICOM_OK;
 }
 
 /* Helper functions for elemement2value for integer and real types.
@@ -286,7 +313,8 @@ int element2simplevalueHelper2(octave_value *ov, const gdcm::DataElement *elem,
 template <gdcm::VR::VRType vrtype>
 static inline 
 int element2intvalueHelper(octave_value *ov, const gdcm::DataElement * elem,
-			   const int chatty) {
+         const int chatty)
+{
   typedef typename gdcm::VRToType<vrtype >::Type valueType;
   return element2simplevalueHelper2<vrtype,valueType,intNDArray<octave_int<valueType> > >(ov, elem, chatty);
 }
@@ -295,230 +323,306 @@ int element2intvalueHelper(octave_value *ov, const gdcm::DataElement * elem,
 template <gdcm::VR::VRType vrtype>
 static inline 
 int element2realvalueHelper(octave_value *ov, const gdcm::DataElement * elem,
-			   const int chatty) {
+         const int chatty)
+{
   typedef typename gdcm::VRToType<vrtype >::Type valueType;
   return element2simplevalueHelper2<vrtype,valueType,Array<valueType> >(ov, elem, chatty);
 }
 
 int element2value(std::string & varname, octave_value *ov, const gdcm::DataElement * elem, 
-				int chatty, int sequenceDepth) {
-	// get dicom dictionary
-	//static const gdcm::Global& g = gdcm::Global::GetInstance();
-	//static const gdcm::Dicts &dicts = g.GetDicts();
-	const gdcm::Tag tag = elem->GetTag();		
-	gdcm::VR vr = elem->GetVR(); // value representation
-
-	// skip "Group Length" tags. note: these are deprecated in DICOM 2008
-	if(tag.GetElement() == (uint16_t)0 || (elem->GetByteValue() == NULL && vr != gdcm::VR::SQ)) 
-	  return DICOM_NOTHING_ASSIGNED;
-	//const gdcm::DictEntry dictEntry = dicts.GetDictEntry(tag,(const char*)0);
-
-	// find dictionary entry for name and to possibly adjust the VR
-	gdcm::DictEntry dictEntry ;
-	if (!dicom_is_present(tag)) {
-		char fallbackVarname[64];
-		snprintf(fallbackVarname,63,"Private_%04x_%04x",tag.GetGroup(),tag.GetElement());
-		varname=std::string(fallbackVarname);
-	}
-	else {
-	        lookup_dicom_entry(dictEntry, tag);
-		varname=dictEntry.GetName();
-		const gdcm::VR dictvr= dictEntry.GetVR(); // value representation. ie DICOM 	
-		if (!vr.Compatible(vr)) {
-    		  warning(QUOTED(OCT_FN_NAME)": %s has different VR from dictionary. Using VR from file.", varname.c_str());
-		}
-		// find vr from dictionary if it was not in the file
-		// lines copied from gdcmPrinter::PrintDataElement
-		if( vr == gdcm::VR::INVALID )
-		  {
-		    vr = dictvr;
-		  }
-		else if ( vr == gdcm::VR::UN && vr != gdcm::VR::INVALID ) // File is explicit, but still prefer vr from dict when UN
-		  {
-		    vr = dictvr;
-		  }
-		else // cool the file is Explicit !
-		  {
-		    // keep vr from file
-		  }
+        int chatty, int sequenceDepth)
+{
+  // get dicom dictionary
+  //static const gdcm::Global& g = gdcm::Global::GetInstance();
+  //static const gdcm::Dicts &dicts = g.GetDicts();
+  const gdcm::Tag tag = elem->GetTag ();   
+  gdcm::VR vr = elem->GetVR (); // value representation
+
+  // skip "Group Length" tags. note: these are deprecated in DICOM 2008
+  if(tag.GetElement() == (uint16_t)0 || (elem->GetByteValue() == NULL && vr != gdcm::VR::SQ)) 
+    return DICOM_NOTHING_ASSIGNED;
+  //const gdcm::DictEntry dictEntry = dicts.GetDictEntry(tag,(const char*)0);
+
+  // find dictionary entry for name and to possibly adjust the VR
+  gdcm::DictEntry dictEntry ;
+  if (!dicom_is_present(tag))
+    {
+      char fallbackVarname[64];
+      snprintf (fallbackVarname, 63, "Private_%04x_%04x", tag.GetGroup(), tag.GetElement());
+      varname = std::string (fallbackVarname);
+    }
+  else
+    {
+      lookup_dicom_entry (dictEntry, tag);
+      varname = dictEntry.GetName ();
+      const gdcm::VR dictvr = dictEntry.GetVR(); // value representation. ie DICOM   
+      if (!vr.Compatible(vr))
+        {
+          warning (QUOTED(OCT_FN_NAME)": %s has different VR from dictionary. Using VR from file.", varname.c_str());
+        }
+      // find vr from dictionary if it was not in the file
+      // lines copied from gdcmPrinter::PrintDataElement
+      if (vr == gdcm::VR::INVALID)
+        {
+          vr = dictvr;
+        }
+      else if (vr == gdcm::VR::UN && vr != gdcm::VR::INVALID)
+        {
+          // File is explicit, but still prefer vr from dict when UN
+          vr = dictvr;
+        }
+      else // cool the file is Explicit !
+        {
+          // keep vr from file
+        }
 #if 0
-		// gdcmPrinter goes on with the following fix for the 
-		// case where the VR derived from the dictionary is 'dual' (e.g. US_SS).
-		// However, we cannot do it here as we don't have 
-		// the FILE* F nor the dataset. 
-		// This means that these fields will not be assigned.
-		if( vr.IsDual() ) // This means vr was read from a dict entry:
-		  {
-		    vr = DataSetHelper::ComputeVR(*F,ds, tag);
-		  }
+      // gdcmPrinter goes on with the following fix for the 
+      // case where the VR derived from the dictionary is 'dual' (e.g. US_SS).
+      // However, we cannot do it here as we don't have 
+      // the FILE* F nor the dataset. 
+      // This means that these fields will not be assigned.
+      if (vr.IsDual()) // This means vr was read from a dict entry:
+        {
+          vr = DataSetHelper::ComputeVR(*F,ds, tag);
+        }
 #endif
-	}
-
-
-	//int tagVarNameBufLen=127;
-	//char *keyword=(char *)malloc((tagVarNameBufLen+1)*sizeof(char));
-	//keyword=name2Keyword(keyword,&tagVarNameBufLen,tagName);
-	//*varname=std::string(keyword);
-	
-	if(chatty) {
-		octave_stdout << std::setw(2*sequenceDepth) << "" << std::setw(0) ;
-		octave_stdout << tag << ":" << vr << ":" << varname << ":" ;
-		// TODO: error if var name repeated. 
-	}
+    }
+
+  //int tagVarNameBufLen=127;
+  //char *keyword=(char *)malloc((tagVarNameBufLen+1)*sizeof(char));
+  //keyword=name2Keyword(keyword,&tagVarNameBufLen,tagName);
+  //*varname=std::string(keyword);
+  
+  if(chatty)
+    {
+      octave_stdout << std::setw(2*sequenceDepth) << "" << std::setw(0) ;
+      octave_stdout << tag << ":" << vr << ":" << varname << ":" ;
+      // TODO: error if var name repeated. 
+    }
 #define strValBufLen 511
-	char strValBuf[strValBufLen+1];
-	char* strVal=strValBuf;
-	
-	if ( vr & gdcm::VR::VRASCII) {
-		strVal=byteval2string(strValBuf,strValBufLen,elem->GetByteValue()); 
-		if(chatty) {
-			if (dicom_truncate_numchar>0) {
-				octave_stdout << '[' << std::string(strVal).substr(0,dicom_truncate_numchar) 
-					<< ( ((int)strlen(strVal)>dicom_truncate_numchar) ? "..." : "") << ']' << std::endl;
-			} else {
-				octave_stdout << '[' << strVal << ']' << std::endl;
-			}
-		}
-		if (vr & VRSTRING) { //all straight to string types
-			*ov=std::string(strVal); // TODO: error if om already has member with this name
-		} else if (vr & gdcm::VR::IS) { // Integer String. spec tallies with signed 32 bit int
-			*ov=(int32_t)atoi(strVal); 
-		} else if (vr & gdcm::VR::DS) { // Double String. vector separated by '/'
-			Matrix vec=str2DoubleVec(strVal);
-			*ov=vec;
-		} else {
-			if(chatty) octave_stdout << "   ### string type not handled ###" << std::endl;
-			return DICOM_NOTHING_ASSIGNED;
-		}
-		if (strVal != strValBuf) free(strVal); // long string. malloc'd instead of using buf, now needs free'ng
-	} else if (vr & gdcm::VR::SQ) {
-		if(chatty) octave_stdout << " reading sequence. "; // \n provided in dumpSequence fn
-		gdcm::SmartPointer<gdcm::SequenceOfItems> sqi = elem->GetValueAsSQ();
-		dumpSequence(ov, sqi, chatty, sequenceDepth+1);
-	} else if (vr & gdcm::VR::AT) { // attribute tag
-		intNDArray<octave_uint16> uint16pair(dim_vector(1,2));
-		uint16_t *p=(uint16_t *)elem->GetByteValue()->GetPointer();
-                uint16pair(0) = p[0];
-                uint16pair(1) = p[1];
-		*ov=uint16pair;
-		if (chatty) {
-			char buf[16];
-			snprintf(buf,15,"[(%04X,%04X)]\n",p[0],p[1]);
-			octave_stdout << buf  ;
-		}
-	} else if (vr & gdcm::VR::FL) {// float
-	  return element2realvalueHelper<gdcm::VR::FL>(ov, elem, chatty);
-	} else if (vr & gdcm::VR::FD) {// double
-	  return element2realvalueHelper<gdcm::VR::FD>(ov, elem, chatty);
-	} else if (vr & gdcm::VR::UL) {// unsigned long
-	  return element2intvalueHelper<gdcm::VR::UL>(ov, elem, chatty);
-	} else if (vr & gdcm::VR::SL) {// signed long
-	  return element2intvalueHelper<gdcm::VR::SL>(ov, elem, chatty);
-	} else if (vr & gdcm::VR::US) {// unsigned short
-	  return element2intvalueHelper<gdcm::VR::US>(ov, elem, chatty);
-	} else if (vr & gdcm::VR::SS) {// signed short
-	  return element2intvalueHelper<gdcm::VR::SS>(ov, elem, chatty);
-	} else if (vr & gdcm::VR::OB) {// other byte
-		if (tag==gdcm::Tag(0x7FE0,0x0010)) { // PixelData
-			if(chatty) octave_stdout  << "skipping, leave for dicomread\n";
-			return DICOM_NOTHING_ASSIGNED;
-		}
-		const uint32_t len=elem->GetByteValue()->GetLength();
-		intNDArray<octave_uint8> bytearr(dim_vector(1,len));
-		const uint8_t *p=(uint8_t *)elem->GetByteValue()->GetPointer();
-                for (uint32_t i=0; i<len; i++)
-                  {
-                     bytearr(i) = p[i];
-                  }
-		*ov=bytearr;
-		if (chatty) {
-			uint32_t i;
-			char buf[8];
-			octave_stdout  << '[' ;
-			for (i=0; i<len; i++) {
-				snprintf(buf,7,"%02X ",(const uint8_t)p[i]);
-				octave_stdout  << buf << " " ;
-			}
-			octave_stdout  << "]\n";
-		}
-	} else {
-		if(chatty) octave_stdout << "   ### VR not handled ###" << std::endl;
-		return DICOM_NOTHING_ASSIGNED;
-	}
-	//free(keyword);
-	return DICOM_OK;
+  char strValBuf[strValBufLen+1];
+  char* strVal=strValBuf;
+  
+  if ( vr & gdcm::VR::VRASCII)
+    {
+      strVal = byteval2string (strValBuf, strValBufLen, elem->GetByteValue ()); 
+      if(chatty)
+        {
+          if (dicom_truncate_numchar > 0)
+            {
+              octave_stdout << '[' << std::string(strVal).substr(0,dicom_truncate_numchar) 
+                << ( ((int)strlen(strVal)>dicom_truncate_numchar) ? "..." : "") << ']' << std::endl;
+            }
+          else
+            {
+              octave_stdout << '[' << strVal << ']' << std::endl;
+            }
+        }
+      if (vr & VRSTRING)
+        {
+          //all straight to string types
+          *ov = std::string(strVal); // TODO: error if om already has member with this name
+        }
+      else if (vr & gdcm::VR::IS)
+        {
+          // Integer String. spec tallies with signed 32 bit int
+          *ov = (int32_t)atoi(strVal); 
+        }
+      else if (vr & gdcm::VR::DS)
+        {
+          // Double String. vector separated by '/'
+          Matrix vec=str2DoubleVec(strVal);
+          *ov=vec;
+        }
+      else
+        {
+          if(chatty)
+            octave_stdout << "   ### string type not handled ###" << std::endl;
+          return DICOM_NOTHING_ASSIGNED;
+        }
+      if (strVal != strValBuf)
+        free(strVal); // long string. malloc'd instead of using buf, now needs free'ng
+    }
+  else if (vr & gdcm::VR::SQ)
+    {
+      if(chatty)
+        octave_stdout << " reading sequence. "; // \n provided in dumpSequence fn
+
+      gdcm::SmartPointer<gdcm::SequenceOfItems> sqi = elem->GetValueAsSQ ();
+      if(sqi)
+        dumpSequence (ov, sqi, chatty, sequenceDepth+1);
+      else
+        *ov = octave_map ();
+    }
+  else if (vr & gdcm::VR::AT)
+    {
+      // attribute tag
+      intNDArray<octave_uint16> uint16pair(dim_vector(1,2));
+      uint16_t *p = (uint16_t *)elem->GetByteValue()->GetPointer();
+      uint16pair(0) = p[0];
+      uint16pair(1) = p[1];
+      *ov=uint16pair;
+      if (chatty)
+        {
+          char buf[16];
+          snprintf (buf, 15, "[(%04X,%04X)]\n", p[0], p[1]);
+          octave_stdout << buf;
+         }
+    }
+  else if (vr & gdcm::VR::FL)
+    {
+      // float
+      return element2realvalueHelper<gdcm::VR::FL>(ov, elem, chatty);
+    }
+  else if (vr & gdcm::VR::FD)
+    {
+      // double
+      return element2realvalueHelper<gdcm::VR::FD>(ov, elem, chatty);
+    }
+  else if (vr & gdcm::VR::UL)
+    {
+      // unsigned long
+      return element2intvalueHelper<gdcm::VR::UL>(ov, elem, chatty);
+    }
+  else if (vr & gdcm::VR::SL)
+    {
+      // signed long
+      return element2intvalueHelper<gdcm::VR::SL>(ov, elem, chatty);
+    }
+  else if (vr & gdcm::VR::US)
+    {
+      // unsigned short
+      return element2intvalueHelper<gdcm::VR::US>(ov, elem, chatty);
+    }
+  else if (vr & gdcm::VR::SS)
+    {
+      // signed short
+      return element2intvalueHelper<gdcm::VR::SS>(ov, elem, chatty);
+    }
+  else if (vr & gdcm::VR::OB)
+    {
+      // other byte
+      if (tag == gdcm::Tag(0x7FE0,0x0010))
+        { // PixelData
+          if(chatty)
+            octave_stdout  << "skipping, leave for dicomread\n";
+          return DICOM_NOTHING_ASSIGNED;
+        }
+      const uint32_t len = elem->GetByteValue()->GetLength();
+      intNDArray<octave_uint8> bytearr(dim_vector(1,len));
+      const uint8_t *p = (uint8_t *)elem->GetByteValue()->GetPointer();
+      for (uint32_t i = 0; i < len; i++)
+        {
+          bytearr(i) = p[i];
+        }
+      *ov = bytearr;
+      if (chatty)
+        {
+          uint32_t i;
+          char buf[8];
+          octave_stdout  << '[';
+          for (i = 0; i < len; i++)
+            {
+              snprintf (buf, 7, "%02X ", (const uint8_t)p[i]);
+              octave_stdout  << buf << " " ;
+            }
+          octave_stdout  << "]\n";
+        }
+    }
+  else
+    {
+      if(chatty)
+        octave_stdout << "   ### VR not handled ###" << std::endl;
+      return DICOM_NOTHING_ASSIGNED;
+    }
+  //free(keyword);
+  return DICOM_OK;
 }
 
-void dumpSequence(octave_value *ov, gdcm::SequenceOfItems *seq, int chatty, int sequenceDepth) {
-	octave_map om;
-	octave_idx_type item_no = 0;
-	for (gdcm::SequenceOfItems::Iterator sit=seq->Begin(); sit != seq->End(); sit++) {
-		gdcm::DataSet ds=sit->GetNestedDataSet();
-		octave_map subom;
-		std::ostringstream item_name_buf;
-		gdcm::Item mi = *sit;
-		item_no ++;
-		if (chatty) octave_stdout << "object " << item_no << " " << mi.GetTag() << std::endl;
-		for (gdcm::DataSet::Iterator dit=ds.Begin(); dit != ds.End(); dit++) {
-			std::string key("");
-			octave_value subov;
-			if( DICOM_OK==element2value(key, &subov, &(*dit), chatty, sequenceDepth)) {
-				subom.assign(key.c_str(), subov);
-			} else {
-				if (0==key.length()) continue ;
-				subom.assign(key.c_str(), octave_value("not assigned"));
-			}
-			if (chatty) octave_stdout << "object sub map " << subom.nfields() << std::endl;
-		}
-		item_name_buf << "Item_" << item_no;
-		om.assign(item_name_buf.str().c_str(), octave_value(subom));
-	}
-	*ov=om;
+void dumpSequence (octave_value *ov, gdcm::SequenceOfItems *seq, int chatty, int sequenceDepth)\
+{
+  octave_map om;
+  octave_idx_type item_no = 0;
+  for (gdcm::SequenceOfItems::Iterator sit = seq->Begin(); sit != seq->End(); sit++)
+    {
+      gdcm::DataSet ds = sit->GetNestedDataSet();
+      octave_map subom;
+      std::ostringstream item_name_buf;
+      gdcm::Item mi = *sit;
+      item_no ++;
+      if (chatty)
+        octave_stdout << "object " << item_no << " " << mi.GetTag() << std::endl;
+      for (gdcm::DataSet::Iterator dit = ds.Begin(); dit != ds.End(); dit++)
+        {
+          std::string key("");
+          octave_value subov;
+          if (DICOM_OK == element2value(key, &subov, &(*dit), chatty, sequenceDepth))
+            {
+              subom.assign (key.c_str(), subov);
+            }
+          else
+            {
+              if (0 == key.length ())
+                continue ;
+              subom.assign (key.c_str(), octave_value("not assigned"));
+            }
+          if (chatty)
+            octave_stdout << "object sub map " << subom.nfields() << std::endl;
+        }
+      item_name_buf << "Item_" << item_no;
+      om.assign (item_name_buf.str().c_str(), octave_value (subom));
+    }
+  *ov = om;
 }
 
-void getFileModTime(char *timeStr, const char *filename) {
-	struct tm* clock;				// create a time structure
-	struct stat attrib;			// create a file attribute structure
-	stat(filename, &attrib);		// get the attributes of afile.txt
-	clock = gmtime(&(attrib.st_mtime));	// Get the last modified time and put it into the time structure
-	char monthStr[4];
-	switch(clock->tm_mon) {
-		case  0: strcpy(monthStr,"Jan"); break;
-		case  1: strcpy(monthStr,"Feb"); break;
-		case  2: strcpy(monthStr,"Mar"); break;
-		case  3: strcpy(monthStr,"Apr"); break;
-		case  4: strcpy(monthStr,"May"); break;
-		case  5: strcpy(monthStr,"Jun"); break;
-		case  6: strcpy(monthStr,"Jul"); break;
-		case  7: strcpy(monthStr,"Aug"); break;
-		case  8: strcpy(monthStr,"Sep"); break;
-		case  9: strcpy(monthStr,"Oct"); break;
-		case 10: strcpy(monthStr,"Nov"); break;
-		case 11: strcpy(monthStr,"Dec"); break;
-	}
-	snprintf(timeStr, TIME_STR_LEN, "%02i-%s-%i %02i:%02i:%02i",
-		clock->tm_mday,monthStr,1900+clock->tm_year, 
-		clock->tm_hour, clock->tm_min, clock->tm_sec);
-	//clock->tm_year returns the year (since 1900)
-	// clock->tm_mon returns the month (January = 0)
-	// clock->tm_mday returns the day of the month
-	// 18-Dec-2000 11:06:43
+void getFileModTime (char *timeStr, const char *filename)
+{
+  struct tm* clock;       // create a time structure
+  struct stat attrib;     // create a file attribute structure
+  stat (filename, &attrib);    // get the attributes of afile.txt
+  clock = gmtime (&(attrib.st_mtime)); // Get the last modified time and put it into the time structure
+  char monthStr[4];
+  switch(clock->tm_mon)
+    {
+      case  0: strcpy(monthStr,"Jan"); break;
+      case  1: strcpy(monthStr,"Feb"); break;
+      case  2: strcpy(monthStr,"Mar"); break;
+      case  3: strcpy(monthStr,"Apr"); break;
+      case  4: strcpy(monthStr,"May"); break;
+      case  5: strcpy(monthStr,"Jun"); break;
+      case  6: strcpy(monthStr,"Jul"); break;
+      case  7: strcpy(monthStr,"Aug"); break;
+      case  8: strcpy(monthStr,"Sep"); break;
+      case  9: strcpy(monthStr,"Oct"); break;
+      case 10: strcpy(monthStr,"Nov"); break;
+      case 11: strcpy(monthStr,"Dec"); break;
+    }
+  snprintf (timeStr, TIME_STR_LEN, "%02i-%s-%i %02i:%02i:%02i",
+    clock->tm_mday,monthStr,1900+clock->tm_year, 
+    clock->tm_hour, clock->tm_min, clock->tm_sec);
+  //clock->tm_year returns the year (since 1900)
+  // clock->tm_mon returns the month (January = 0)
+  // clock->tm_mday returns the day of the month
+  // 18-Dec-2000 11:06:43
 }
 
-Matrix str2DoubleVec(const char* s){
-	// count separators, hence elements
-	int n=1;
-	char *sl=(char *)s; 
-	for (; *sl != '\0'; sl++) n = (*sl == '\\') ? n+1 : n;
-	// create output args
-	Matrix dv(n, 1);
-	double *fv=dv.fortran_vec();
-	// parse into output
-	int i=0;
-	for (sl=(char *)s; i<n ; i++, sl++) {
-		fv[i]=strtod(sl,&sl);
-	}
-	return dv;
+Matrix str2DoubleVec (const char* s)
+{
+  // count separators, hence elements
+  int n=1;
+  char *sl=(char *)s; 
+  for (; *sl != '\0'; sl++)
+    n = (*sl == '\\') ? n+1 : n;
+  // create output args
+  Matrix dv(n, 1);
+  double *fv=dv.fortran_vec();
+  // parse into output
+  int i=0;
+  for (sl=(char *)s; i<n ; i++, sl++)
+    {
+      fv[i] = strtod (sl,&sl);
+    }
+  return dv;
 }
 
 
@@ -527,18 +631,22 @@ Matrix str2DoubleVec(const char* s){
 // place as the supplied, the returned pointer should be freed.
 // returned pointer, as the supplied one may be invalid.
 // d_len: length of d.
-char* byteval2string(char * d, int d_len, const gdcm::ByteValue *bv) {
-	if(bv==NULL) { // make a null string, "" 
-		*d='\0';
-		return d;
-	}
-	int len = bv->GetLength();
-	if ( len > d_len ) {
-		d=(char *)malloc((len+1)*sizeof(char));
-	}
-	memcpy(d, bv->GetPointer(), len);
-	d[len]='\0';
-	return d;
+char* byteval2string (char * d, int d_len, const gdcm::ByteValue *bv)
+{
+  if (bv == NULL)
+    { 
+      // make a null string, "" 
+      *d='\0';
+      return d;
+    }
+  int len = bv->GetLength ();
+  if (len > d_len)
+    {
+      d=(char *)malloc ((len+1)*sizeof(char));
+    }
+  memcpy (d, bv->GetPointer(), len);
+  d[len]='\0';
+  return d;
 }
 
 // remove non-alphabet characters from a string.
@@ -547,24 +655,32 @@ char* byteval2string(char * d, int d_len, const gdcm::ByteValue *bv) {
 // this fn will realloc if it is not big enough. so use
 // returned pointer, as the supplied one may be invalid.
 // d_len_p: pointer to length of d. is updated if required.
-char* name2Keyword(char *d, int *d_len_p, const char* s) {
-	char *f=(char*)s; //from (loop through source)
-	int len=strlen(s);
-	if ( len > *d_len_p ) {
-		d=(char *)realloc(d,(len+1)*sizeof(char));
-	}
-	char *tl=(char*)d; // pointer to loop through the destination
-	for (; *f != '\0' ; f++ ) {
-		if ( (*f >= 'A' && *f <= 'Z') || (*f >= 'a' && *f <= 'z') ) {
-			*tl++ = *f;
-		} else if (*f=='\'' && *(f+1)=='s') {
-			f++; // if quote followed by s, skip both chars
-		} else if (*f==' ' && *(f+1) >= 'a' && *(f+1) <= 'z') {
-			*tl++ = *++f - ('a'-'A') ; // space folowed by lower case char, cap char
-		}
-	}
-	*tl = '\0';
-	return d;
+char* name2Keyword (char *d, int *d_len_p, const char* s)
+{
+  char *f=(char*)s; //from (loop through source)
+  int len=strlen(s);
+  if (len > *d_len_p)
+    {
+      d = (char *)realloc (d,(len+1)*sizeof(char));
+    }
+  char *tl=(char*)d; // pointer to loop through the destination
+  for (; *f != '\0' ; f++ )
+    {
+      if ( (*f >= 'A' && *f <= 'Z') || (*f >= 'a' && *f <= 'z') )
+        {
+          *tl++ = *f;
+        }
+      else if (*f == '\'' && *(f+1)=='s')
+        {
+          f++; // if quote followed by s, skip both chars
+        }
+      else if (*f == ' ' && *(f+1) >= 'a' && *(f+1) <= 'z')
+        {
+          *tl++ = *++f - ('a'-'A') ; // space folowed by lower case char, cap char
+        }
+     }
+  *tl = '\0';
+  return d;
 }
 
 /*
diff --git a/src/dicomlookup.cpp b/src/dicomlookup.cpp
index a4c6623..8c2a816 100644
--- a/src/dicomlookup.cpp
+++ b/src/dicomlookup.cpp
@@ -39,7 +39,7 @@
 // build_against_gdcm dicomlookup.cpp dicomdict.cpp -o dicomlookup.oct
 
 DEFUN_DLD (OCT_FN_NAME_LU, args, nargout,
-		"-*- texinfo -*- \n\
+    "-*- texinfo -*- \n\
 @deftypefn {Loadable Function} @var{keyword} = dicomlookup (@var{group}, @var{element}) \n\
 @deftypefnx {Loadable Function} [@var{group}, @var{element}] = dicomlookup (@var{keyword}) \n\
 \n\
@@ -57,39 +57,47 @@ for keyword.\n\
 @end deftypefn \n\
 ")
 {
-	octave_value_list retval;  // create object to store return values
-	if (args.length()==1) { // keyword to tag
-		charMatrix arg0mat = args(0).char_matrix_value ();
-		if (arg0mat.rows()!=1) {
-			error(QUOTED(OCT_FN_NAME_LU)": first arg should be a single row of chars: a string containing a DICOM keyword");
-			return retval;
-		}
-		std::string keyword = arg0mat.row_as_string (0);
-		gdcm::Tag tag;
-		lookup_dicom_tag(tag, keyword);
-		octave_uint16 group=tag.GetGroup();
-		octave_uint16 elem=tag.GetElement();
-		retval(0)=octave_value(group);
-		retval(1)=octave_value(elem);
-		return retval;
-	}
-	if (args.length()==2) { // tag to keyword 
-		uint16_t tagvals[2];
-		for( int i=0 ; i<2 ; i++) {
-			if (args(i).is_string()) {
-				std::istringstream iss(args(i).char_matrix_value().row_as_string(0));
-				iss >> std::setbase(16) >> tagvals[i];
-			} else {
-				tagvals[i] = args(i).int_vector_value().fortran_vec()[0] ;
-			}
-		}
-		gdcm::Tag tag(tagvals[0], tagvals[1]);
-		std::string keyword;
-		lookup_dicom_keyword(keyword, tag);
-		return octave_value(keyword);
-	}
-	error(QUOTED(OCT_FN_NAME_LU)": takes 1 or 2 arguments, got %i. see help", (int)args.length ());
-	return retval;
+  octave_value_list retval;  // create object to store return values
+  if (args.length() == 1)
+    { // keyword to tag
+      charMatrix arg0mat = args(0).char_matrix_value ();
+      if (arg0mat.rows () != 1)
+        {
+          error (QUOTED(OCT_FN_NAME_LU)": first arg should be a single row of chars: a string containing a DICOM keyword");
+          return retval;
+        }
+      std::string keyword = arg0mat.row_as_string (0);
+      gdcm::Tag tag;
+      lookup_dicom_tag(tag, keyword);
+      octave_uint16 group = tag.GetGroup();
+      octave_uint16 elem = tag.GetElement();
+      retval(0) = octave_value(group);
+      retval(1) = octave_value(elem);
+      return retval;
+    }
+  if (args.length()==2)
+    {
+      // tag to keyword 
+      uint16_t tagvals[2];
+      for (int i = 0 ; i < 2 ; i++)
+        {
+          if (args(i).is_string())
+            {
+              std::istringstream iss(args(i).char_matrix_value().row_as_string(0));
+              iss >> std::setbase(16) >> tagvals[i];
+            }
+          else
+            {
+              tagvals[i] = args(i).int_vector_value().fortran_vec()[0] ;
+            }
+         }
+       gdcm::Tag tag (tagvals[0], tagvals[1]);
+       std::string keyword;
+       lookup_dicom_keyword (keyword, tag);
+       return octave_value (keyword);
+     }
+  error (QUOTED(OCT_FN_NAME_LU)": takes 1 or 2 arguments, got %i. see help", (int)args.length ());
+  return retval;
 }
 
 /*
diff --git a/src/dicomread.cpp b/src/dicomread.cpp
index e38b3f5..dec6d4c 100644
--- a/src/dicomread.cpp
+++ b/src/dicomread.cpp
@@ -40,122 +40,152 @@ using namespace gdcm;
 #define QUOTED(x) QUOTED_(x)
 
 DEFUN_DLD (dicomread, args, nargout,
-		"-*- texinfo -*- \n\
- @deftypefn {Loadable Function} @var{image} = dicomread (@var{filename}) \n\
- @deftypefnx {Loadable Function} @var{image} = dicomread (@var{structure}) \n\
- \n\
- Load the image from a DICOM file. \n\
- @var{filename} is a string (giving the filename).\n\
- @var{structure} is a structure with a field @code{Filename} (such as returned by @code{dicominfo}).\n\
- @var{image} may be two or three dimensional, depending on the content of the file. \n\
- An integer or float matrix will be returned, the number of bits will depend on the file. \n\
+  "-*- texinfo -*- \n\
+@deftypefn {Loadable Function} @var{image} = dicomread (@var{filename}) \n\
+@deftypefnx {Loadable Function} @var{image} = dicomread (@var{structure}) \n\
 \n\
- @seealso{dicominfo} \n\
- @end deftypefn \n\
- ")
+Load the image from a DICOM file. \n\
+@var{filename} is a string (giving the filename).\n\
+@var{structure} is a structure with a field @code{Filename} (such as returned by @code{dicominfo}).\n\
+@var{image} may be two or three dimensional, depending on the content of the file. \n\
+An integer or float matrix will be returned, the number of bits will depend on the file. \n\
+\n\
+@seealso{dicominfo} \n\
+@end deftypefn \n\
+")
 {
-	octave_value_list retval;  // create object to store return values
-	if ( 0 == args.length()) {
-		error(QUOTED(OCT_FN_NAME)": one arg required: dicom filename");
-		return retval; 
-	}
-
-	std::string filename;
-	// argument processing
-	// check if 1st argument is a string or a struct with field Filename
-	// If so, assign to filename variable, otherwise exit.
-	if (args(0).is_string()) {
-	  filename = args(0).string_value();
-	}
-	else {
-          if (! args(0).OV_ISMAP ()) {
-	        error(QUOTED(OCT_FN_NAME)": arg should be a filename, 1 row of chars, or a struct returned by dicominfo");
-		return retval; 
-	  }
-	  octave_scalar_map arg0 = args(0).scalar_map_value ();
-	  if (!arg0.contains("Filename")) {
-	        error(QUOTED(OCT_FN_NAME)": if arg is a struct, it should have the Filename field");
-		return retval; 
-	  }
-	  octave_value tmp = arg0.getfield ("Filename");
-	  filename = tmp.string_value();
-	}
-		
-	
+  octave_value_list retval;  // create object to store return values
+  if ( 0 == args.length()) 
+    {
+      error (QUOTED(OCT_FN_NAME)": one arg required: dicom filename");
+      return retval; 
+    }
+
+  std::string filename;
+  // argument processing
+  // check if 1st argument is a string or a struct with field Filename
+  // If so, assign to filename variable, otherwise exit.
+  if (args(0).is_string ())
+    {
+      filename = args(0).string_value();
+    }
+  else
+    {
+      if (! args(0).OV_ISMAP ())
+        {
+          error(QUOTED(OCT_FN_NAME)": arg should be a filename, 1 row of chars, or a struct returned by dicominfo");
+          return retval; 
+        }
+      octave_scalar_map arg0 = args(0).scalar_map_value ();
+      if (!arg0.contains("Filename"))
+        {
+          error (QUOTED(OCT_FN_NAME)": if arg is a struct, it should have the Filename field");
+          return retval; 
+        }
+      octave_value tmp = arg0.getfield ("Filename");
+      filename = tmp.string_value();
+    }
+    
 #if 0 /* TODO support 'frames' stuff, see Matlab docs for dicomread */
-	int i; // parse any additional args
-	for (i=1; i<args.length(); i++) {
-		charMatrix chex = args(i).char_matrix_value();
-		if (chex.rows()!=1) {
-			error(QUOTED(OCT_FN_NAME)": arg should be a string, 1 row of chars");
-			return retval; 
-		}
-	}
+  int i; // parse any additional args
+  for (i=1; i<args.length(); i++)
+    {
+      charMatrix chex = args(i).char_matrix_value();
+      if (chex.rows() != 1)
+        {
+          error (QUOTED(OCT_FN_NAME)": arg should be a string, 1 row of chars");
+          return retval; 
+        }
+    }
 #endif
-	
-	gdcm::ImageReader reader;
-	reader.SetFileName( filename.c_str() );
-	if( !reader.Read() ) {
-		error(QUOTED(OCT_FN_NAME)": Could not read DICOM file with image: %s",filename.c_str());
-		return retval;
-	}
-	
-	const gdcm::Image &image = reader.GetImage();
-
-	const octave_idx_type ndim = image.GetNumberOfDimensions();
-	const unsigned int * const dims = image.GetDimensions();
-	// dim 0: cols (width)
-	// dim 1: rows (height)
-	// dim 2: number of frames
-
-	dim_vector dv;
-	Array<octave_idx_type> perm_vect(dim_vector(ndim,1));
-	perm_vect(0) = 1;
-	perm_vect(1) = 0;
-
-	// TODO check with non-square images if this needs to be dims[1],dims[0] etc
-	if( 2==ndim ) {
-		//this transposes first two dimensions
-		dv = dim_vector(octave_idx_type(dims[0]), octave_idx_type(dims[1]));
-	} else if (3==ndim) {
-		// should be (rows, cols, pages) in octave idiom
-		dv = dim_vector(octave_idx_type(dims[0]), octave_idx_type(dims[1]), octave_idx_type(dims[2])); 
-		perm_vect(2)=2;
-	} else {
-		error(QUOTED(OCT_FN_NAME)": %i dimensions. not supported: %s",(int)ndim, filename.c_str());
-		return retval;
-	}
-
-	if ( gdcm::PixelFormat::UINT32 == image.GetPixelFormat() ) { //tested
-		uint32NDArray arr(dv);
-		image.GetBuffer((char *)arr.fortran_vec());
-		return octave_value(arr.permute(perm_vect));
-	} else if ( gdcm::PixelFormat::UINT16 == image.GetPixelFormat() ) { //tested
-		uint16NDArray arr(dv);
-		image.GetBuffer((char *)arr.fortran_vec());
-		return octave_value(arr.permute(perm_vect));
-	} else if ( gdcm::PixelFormat::UINT8 == image.GetPixelFormat() ) { //tested
-		uint8NDArray arr(dv);
-		image.GetBuffer((char *)arr.fortran_vec());
-		return octave_value(arr.permute(perm_vect));
-	} else if ( gdcm::PixelFormat::INT8 == image.GetPixelFormat() ) { // no example found to test
-		int8NDArray arr(dv);
-		image.GetBuffer((char *)arr.fortran_vec());
-		return octave_value(arr.permute(perm_vect));
-	} else if ( gdcm::PixelFormat::INT16 == image.GetPixelFormat() ) { // no example found to test
-		int16NDArray arr(dv);
-		image.GetBuffer((char *)arr.fortran_vec());
-		return octave_value(arr.permute(perm_vect));
-	} else if ( gdcm::PixelFormat::INT32 == image.GetPixelFormat() ) { // no example found to test
-		int32NDArray arr(dv);
-		image.GetBuffer((char *)arr.fortran_vec());
-		return octave_value(arr.permute(perm_vect));
-	} else {
-		octave_stdout << image.GetPixelFormat() << '\n' ;
-		error(QUOTED(OCT_FN_NAME)": pixel format not supported yet: %s", filename.c_str());
-		return retval;
-	}
-	
+  
+  gdcm::ImageReader reader;
+  reader.SetFileName (filename.c_str());
+  if (!reader.Read())
+    {
+      error (QUOTED(OCT_FN_NAME)": Could not read DICOM file with image: %s",filename.c_str());
+      return retval;
+    }
+  
+  const gdcm::Image &image = reader.GetImage();
+
+  const octave_idx_type ndim = image.GetNumberOfDimensions();
+  const unsigned int * const dims = image.GetDimensions();
+  // dim 0: cols (width)
+  // dim 1: rows (height)
+  // dim 2: number of frames
+
+  dim_vector dv;
+  Array<octave_idx_type> perm_vect(dim_vector(ndim,1));
+  perm_vect(0) = 1;
+  perm_vect(1) = 0;
+
+  // TODO check with non-square images if this needs to be dims[1],dims[0] etc
+  if( 2 == ndim )
+    {
+      //this transposes first two dimensions
+      dv = dim_vector(octave_idx_type(dims[0]), octave_idx_type(dims[1]));
+    }
+  else if (3 == ndim)
+    {
+      // should be (rows, cols, pages) in octave idiom
+      dv = dim_vector(octave_idx_type(dims[0]), octave_idx_type(dims[1]), octave_idx_type(dims[2])); 
+      perm_vect(2) = 2;
+    }
+  else
+    {
+      error(QUOTED(OCT_FN_NAME)": %i dimensions. not supported: %s",(int)ndim, filename.c_str());
+      return retval;
+    }
+
+  if (gdcm::PixelFormat::UINT32 == image.GetPixelFormat())
+    {
+      //tested
+      uint32NDArray arr(dv);
+      image.GetBuffer ((char *)arr.fortran_vec());
+      return octave_value (arr.permute(perm_vect));
+    }
+  else if ( gdcm::PixelFormat::UINT16 == image.GetPixelFormat() )
+    {
+      //tested
+      uint16NDArray arr(dv);
+      image.GetBuffer ((char *)arr.fortran_vec());
+      return octave_value (arr.permute(perm_vect));
+    }
+  else if ( gdcm::PixelFormat::UINT8 == image.GetPixelFormat() )
+    {
+      //tested
+      uint8NDArray arr(dv);
+      image.GetBuffer ((char *)arr.fortran_vec());
+      return octave_value (arr.permute(perm_vect));
+    }
+  else if ( gdcm::PixelFormat::INT8 == image.GetPixelFormat() )
+    { 
+      // no example found to test
+      int8NDArray arr(dv);
+      image.GetBuffer((char *)arr.fortran_vec());
+      return octave_value(arr.permute(perm_vect));
+    }
+  else if ( gdcm::PixelFormat::INT16 == image.GetPixelFormat() )
+    { 
+      // no example found to test
+      int16NDArray arr(dv);
+      image.GetBuffer((char *)arr.fortran_vec());
+      return octave_value(arr.permute(perm_vect));
+    }
+  else if ( gdcm::PixelFormat::INT32 == image.GetPixelFormat() )
+    { 
+      // no example found to test
+      int32NDArray arr(dv);
+      image.GetBuffer((char *)arr.fortran_vec());
+      return octave_value(arr.permute(perm_vect));
+    }
+  else
+    {
+      octave_stdout << image.GetPixelFormat() << '\n' ;
+      error(QUOTED(OCT_FN_NAME)": pixel format not supported yet: %s", filename.c_str());
+      return retval;
+    }
 }
 
 /*
diff --git a/src/dicomuid.cpp b/src/dicomuid.cpp
index 9c6cbee..def1e94 100644
--- a/src/dicomuid.cpp
+++ b/src/dicomuid.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright John Donoghue, 2017:
+ * Copyright John Donoghue, 2017-2020
  *
  * The GNU Octave dicom package is free software: you can redistribute 
  * it and/or modify it under the terms of the GNU General Public 
@@ -34,7 +34,7 @@ using namespace gdcm;
 #define QUOTED(x) QUOTED_(x)
 
 DEFUN_DLD (dicomuid, args, nargout,
-		"-*- texinfo -*- \n\
+    "-*- texinfo -*- \n\
 @deftypefn {Loadable Function} @var{uuid} = dicomuuid () \n\
 \n\
 Generate a DICOM unique id . \n\
@@ -44,19 +44,20 @@ Generate a DICOM unique id . \n\
 @end deftypefn \n\
 ")
 {
-	octave_value_list retval;  // create object to store return values
+  octave_value_list retval;  // create object to store return values
 
-	// no args required
-	if ( 0 != args.length ()) {
-      		print_usage ();
-		return retval; 
-	}
+  // no args required
+  if (0 != args.length ())
+    {
+      print_usage ();
+      return retval; 
+    }
 
-	gdcm::UIDGenerator generator;
+  gdcm::UIDGenerator generator;
 
-	retval = octave_value (generator.Generate ());
+  retval = octave_value (generator.Generate ());
 
-	return retval;
+  return retval;
 }
 
 /*
diff --git a/src/dicomwrite.cpp b/src/dicomwrite.cpp
index 1e044c8..f352f18 100644
--- a/src/dicomwrite.cpp
+++ b/src/dicomwrite.cpp
@@ -49,14 +49,14 @@
 
 // TODO all fns here should throw exceptions, not use this "std::string & err" arg
 
-void struct2metadata(gdcm::ImageWriter *w, gdcm::File *file, const octave_value  & ov, bool trial, int sequenceDepth) ;
-void structarray2sequence(gdcm::SequenceOfItems & sq, octave_map * om, bool trial, int sequenceDepth);
+void struct2metadata (gdcm::ImageWriter *w, gdcm::File *file, const octave_value  & ov, bool trial, int sequenceDepth) ;
+void structarray2sequence (gdcm::SequenceOfItems & sq, octave_map * om, bool trial, int sequenceDepth);
 void value2element (gdcm::DataElement * de, const octave_value * ov, gdcm::Tag * tag, const std::string & keyword, bool trial, bool * handled, int sequenceDepth);
-void octaveVal2dicomImage(gdcm::ImageWriter *w, octave_value *pixval) ;
-void genMinimalMetaData(gdcm::ImageWriter *w, gdcm::File *file);
+void octaveVal2dicomImage (gdcm::ImageWriter *w, octave_value *pixval) ;
+void genMinimalMetaData (gdcm::ImageWriter *w, gdcm::File *file);
 
 DEFUN_DLD (dicomwrite, args, nargout,
-		"-*- texinfo -*- \n\
+    "-*- texinfo -*- \n\
 @deftypefn {Loadable Function} {} dicomwrite(@var{im}, @var{filename})\n\
 @deftypefnx {Loadable Function} {} dicomwrite(@var{im}, @var{filename}, @var{info})\n\
 \n\
@@ -70,417 +70,541 @@ Write a DICOM format file to @var{filename}.\n\
 @end deftypefn \n\
 ")
 {
-	octave_value_list retval;  // create object to store return values
-	if(2>args.length()) {
-		error(QUOTED(OCT_FN_NAME)": should have at least 2 arguments");
-		return retval; 
-	}
-	if (! args(1).is_string ()) {
-		error(QUOTED(OCT_FN_NAME)": second argument should be a string filename");
-		return retval; 
-	}
-
-	charMatrix ch = args(1).char_matrix_value ();
-	bool trial = false;
-	if (0 == ch.numel()) {
-		trial = true; // more output if null string supplied for filename;
-	} else if (ch.rows()!=1) {
-		error(QUOTED(OCT_FN_NAME)": second arg should be a filename");
-		return retval; 
-	}
-	std::string filename = ch.row_as_string(0);
-	
-	//bool fn_failed_verbose=trial; //TODO other way of setting this. is it even necessary?
-	bool writing_image = (0 != args(0).numel()) ;
-		
-	// prepare writer
-	gdcm::ImageWriter w;
-	if (!trial) w.SetFileName( filename.c_str());
-
-	gdcm::SmartPointer<gdcm::File> file = new gdcm::File; // should delete be used later?
-	
-	if (3 > args.length()) { // no metadata supplied, need to make some
-		try {
-			genMinimalMetaData(&w, file);
-		} catch (std::exception&) {
-			return retval ;
-		}
-	} else { // 3rd arg should be struct to turn into metadata
-		try {
-			struct2metadata(&w, file, args(2), trial, 0 /* depth of indent for SQ nesting */);
-		} catch (std::exception&) {
-			return retval ;
-		}
-	}
-
-	if ( writing_image ) { 
-		octave_value pixval = args(0);
-		try {
-			octaveVal2dicomImage(&w, &pixval);
-		} catch (std::exception&) {
-			return retval ;
-		}
-	}
-	
+  octave_value_list retval;  // create object to store return values
+  if (2 > args.length())
+    {
+      error (QUOTED(OCT_FN_NAME)": should have at least 2 arguments");
+      return retval; 
+    }
+  if (! args(1).is_string ())
+    {
+      error (QUOTED(OCT_FN_NAME)": second argument should be a string filename");
+      return retval; 
+    }
+
+  charMatrix ch = args(1).char_matrix_value ();
+  bool trial = false;
+  if (0 == ch.numel())
+    {
+      trial = true; // more output if null string supplied for filename;
+    }
+  else if (ch.rows() != 1)
+    {
+      error (QUOTED(OCT_FN_NAME)": second arg should be a filename");
+      return retval; 
+    }
+  std::string filename = ch.row_as_string(0);
+  
+  //bool fn_failed_verbose=trial; //TODO other way of setting this. is it even necessary?
+  bool writing_image = (0 != args(0).numel()) ;
+    
+  // prepare writer
+  gdcm::ImageWriter w;
+  if (!trial) w.SetFileName (filename.c_str());
+
+  gdcm::SmartPointer<gdcm::File> file = new gdcm::File; // should delete be used later?
+  
+  if (3 > args.length())
+    { 
+      // no metadata supplied, need to make some
+      try
+        {
+          genMinimalMetaData(&w, file);
+        }
+      catch (std::exception&)
+        {
+          return retval;
+        }
+    }
+  else
+    { 
+      // 3rd arg should be struct to turn into metadata
+      try
+        {
+          struct2metadata (&w, file, args(2), trial, 0 /* depth of indent for SQ nesting */);
+        }
+      catch (std::exception&)
+        {
+          return retval;
+        }
+    }
+
+  if (writing_image)
+    { 
+      octave_value pixval = args(0);
+      try
+        {
+          octaveVal2dicomImage(&w, &pixval);
+        }
+      catch (std::exception&)
+        {
+          return retval;
+        }
+     }
+  
 #if 0 /* debug */
-	//NOTE: debug code currently crashes octave - do not use without rework
-	gdcm::File fc=w.GetFile();
-	gdcm::DataSet dsc=fc.GetDataSet();
-	const gdcm::DataElement & sopclass = dsc.GetDataElement( gdcm::Tag(0x0008, 0x0016) ); // SOPClassUID
-	const gdcm::ByteValue *bv = sopclass.GetByteValue();
-	char * bufc=(char *) malloc((bv->GetLength()+1)*sizeof(char));
-	memcpy(bufc, bv->GetPointer(), bv->GetLength());
-	bufc[bv->GetLength()]='\0';
-	octave_stdout << bufc << '\n' ;
-	free(bufc);
+  //NOTE: debug code currently crashes octave - do not use without rework
+  gdcm::File fc=w.GetFile();
+  gdcm::DataSet dsc=fc.GetDataSet();
+  const gdcm::DataElement & sopclass = dsc.GetDataElement( gdcm::Tag(0x0008, 0x0016) ); // SOPClassUID
+  const gdcm::ByteValue *bv = sopclass.GetByteValue();
+  char * bufc=(char *) malloc((bv->GetLength()+1)*sizeof(char));
+  memcpy(bufc, bv->GetPointer(), bv->GetLength());
+  bufc[bv->GetLength()]='\0';
+  octave_stdout << bufc << '\n' ;
+  free(bufc);
 #endif
-	
-	if( !trial ) {
-		// if not writing image, use superclass
-		if (writing_image ? !w.ImageWriter::Write() : !w.Writer::Write()){ 
-			error(QUOTED(OCT_FN_NAME)": unable to save");
-			return retval; 
-		}
-	}
-
-	return retval;
-}
+  
+  if (!trial)
+    {
+      // if not writing image, use superclass
+      if (writing_image ? !w.ImageWriter::Write() : !w.Writer::Write())
+        { 
+          error (QUOTED(OCT_FN_NAME)": unable to save");
+          return retval; 
+        }
+    }
 
-void struct2metadata(gdcm::ImageWriter *w, gdcm::File *file, const octave_value  & ov, bool trial, int sequenceDepth) {
-	if(!ov.OV_ISMAP()){
-		error(QUOTED(OCT_FN_NAME)": 3rd arg should be struct holding metadata. it is %s",ov.type_name().c_str()); 
-		throw std::exception() ;
-	}
-	gdcm::DataSet ds;
-	gdcm::FileMetaInformation hds;
-	octave_map om=ov.map_value();
-	uint32_t skipped = 0;
-	for (octave_map::iterator it = om.begin(); it != om.end(); it++) {
-		std::string keyword(om.key(it));
-		Cell cell = om.contents(it);
-		if (!dicom_is_present(keyword)) {
-			if ( 0==keyword.compare("FileModDate") || 0==keyword.compare("Filename")) {
-				if (trial) octave_stdout << "from dicominfo, ignoring:" << keyword << ":[" << cell(0).string_value() << "]\n";
-				continue; //dicominfo adds these non-DICOM bits
-			}
-			skipped++;
-			// warning(QUOTED(OCT_FN_NAME)": skipping \"%s\". not in dictionary\n",keyword.c_str());
-			continue;
-		}
-		// PixelData is here becuase it handled by other fn. the others seem to cause bugs
-		if ( 0==keyword.compare("PixelData") || 0==keyword.compare("FileMetaInformationVersion") ) {
-			if (trial) octave_stdout << "handled separately:" << keyword << std::endl;
-			continue; 
-		}
-		gdcm::DataElement de;
-		gdcm::Tag tag;
-		bool handled ;
-		try {
-			value2element(&de, &cell(0), &tag, keyword, trial, &handled, sequenceDepth);
-		} catch (std::exception&) {
-			return;
-		}
-		if (handled) {
-			if (tag.GetGroup() == (uint32_t)0x0002 ) { // group 0x2 : header
-				hds.Insert(de) ;
-				// TODO move this somehow: if (trial) octave_stdout << ':' << "(header)";
-			} else { // everything else is metadata
-				ds.Insert(de);
-			}
-		}
-	}
-	if (skipped>0) {
-		// TODO: this does not count elements nested in SQs
-		warning(QUOTED(OCT_FN_NAME)": skipped %u keyword-value pairs. not in dictionary\n",skipped);
-	}
-	// TODO are these set functions taking a copy? if they take a reference to objects that are about to go out of scope, we have a problem
-	file->SetDataSet((gdcm::DataSet &)ds);
-	file->SetHeader(hds);
-	w->SetFile(*file);
-	return ;
+  return retval;
 }
 
-void structarray2sequence(gdcm::SequenceOfItems & sq, octave_map * om, bool trial, int sequenceDepth) {
-	for (octave_map::iterator it = om->begin(); it != om->end(); it++) {
-		gdcm::Item item;
-		// item.SetVLToUndefined(); //TODO: does VL need to be set for items that contain datasets?
-		gdcm::DataSet &nds = item.GetNestedDataSet();
-		std::string itemname(om->key(it));
-		// TODO: test itemname is something like Item_n.
-		Cell cell = om->contents(it);
-		octave_map subom = cell(0).map_value();
-		// octave_stdout << itemname <<std::endl;
-		for (octave_map::iterator subit = subom.begin(); subit != subom.end(); subit++) {
-			std::string subkeyword(subom.key(subit));
-			gdcm::DataElement de;
-			gdcm::Tag tag;
-			bool handled;
-			try {
-				value2element(&de, &(subom.contents(subit)(0)), &tag, subkeyword, trial, &handled, sequenceDepth);
-			} catch (std::exception&) {
-				return;
-			}
-			if (!handled) {
-				warning(QUOTED(OCT_FN_NAME)": element in sequence not handled %s", subkeyword.c_str());
-				continue;
-			}
-			nds.Insert(de); //insert element into dataset
-		}
-		sq.AddItem(item); // add dataset to sequence
-	}
-	return ;
+void struct2metadata (gdcm::ImageWriter *w, gdcm::File *file, const octave_value  & ov, bool trial, int sequenceDepth)
+{
+  if (!ov.OV_ISMAP())
+    {
+      error (QUOTED(OCT_FN_NAME)": 3rd arg should be struct holding metadata. it is %s",ov.type_name().c_str()); 
+      throw std::exception ();
+    }
+  gdcm::DataSet ds;
+  gdcm::FileMetaInformation hds;
+  octave_map om=ov.map_value();
+  uint32_t skipped = 0;
+  for (octave_map::iterator it = om.begin(); it != om.end(); it++)
+    {
+      std::string keyword(om.key(it));
+      Cell cell = om.contents(it);
+      if (!dicom_is_present(keyword))
+        {
+          if ( 0==keyword.compare("FileModDate") || 0==keyword.compare("Filename"))
+            {
+              if (trial)
+                octave_stdout << "from dicominfo, ignoring:" << keyword << ":[" << cell(0).string_value() << "]\n";
+              continue; //dicominfo adds these non-DICOM bits
+            }
+          skipped++;
+          // warning (QUOTED(OCT_FN_NAME)": skipping \"%s\". not in dictionary\n",keyword.c_str());
+          continue;
+        }
+      // PixelData is here becuase it handled by other fn. the others seem to cause bugs
+      if (0 == keyword.compare("PixelData") || 0 == keyword.compare("FileMetaInformationVersion"))
+        {
+          if (trial)
+            octave_stdout << "handled separately:" << keyword << std::endl;
+          continue; 
+        }
+      gdcm::DataElement de;
+      gdcm::Tag tag;
+      bool handled;
+      try
+        {
+          value2element(&de, &cell(0), &tag, keyword, trial, &handled, sequenceDepth);
+        }
+      catch (std::exception&)
+        {
+          return;
+        }
+      if (handled)
+        {
+          if (tag.GetGroup() == (uint32_t)0x0002 )
+            { 
+              // group 0x2 : header
+              hds.Insert(de);
+              // TODO move this somehow: if (trial) octave_stdout << ':' << "(header)";
+            }
+          else
+            { 
+              // everything else is metadata
+              ds.Insert(de);
+            }
+        }
+    }
+  if (skipped > 0)
+    {
+      // TODO: this does not count elements nested in SQs
+      warning (QUOTED(OCT_FN_NAME)": skipped %u keyword-value pairs. not in dictionary\n",skipped);
+    }
+  // TODO are these set functions taking a copy? if they take a reference to objects that are about to go out of scope, we have a problem
+  file->SetDataSet ((gdcm::DataSet &)ds);
+  file->SetHeader (hds);
+  w->SetFile (*file);
+  return;
 }
 
-void value2element (gdcm::DataElement * de, const octave_value * ov, gdcm::Tag * tag, const std::string & keyword, bool trial, bool * handled, int sequenceDepth) {
-	gdcm::DictEntry entry;
-	if (!dicom_is_present(keyword)) {
-		if (trial) {
-			octave_stdout << std::setw(2*sequenceDepth) << "" << std::setw(0) ;
-			octave_stdout << keyword << ": not in dictionary" << std::endl ;
-		}
-		*handled=false;
-		return ;
-	}
-	lookup_dicom_tag(*tag, keyword);
-	lookup_dicom_entry(entry, *tag);
-	gdcm::VL len((uint32_t)ov->byte_size());
-	//gdcm::DataElement de(*tag, len, entry.GetVR()); 
-	de->SetTag(*tag); de->SetVL(len); de->SetVR(entry.GetVR());
-	*handled = true;
-	if (trial) {
-		octave_stdout << std::setw(2*sequenceDepth) << "" << std::setw(0) ;
-		octave_stdout << *tag << ':' << entry.GetVR() << ':' << keyword << ':' ;
-	}
-	if ( entry.GetVR() & VRSTRING ) {// TODO: check even padding requirement
-		// TODO some dicom string types are stored as numbers
-		if (!ov->is_string()) {
-			warning(QUOTED(OCT_FN_NAME)": dicomdict gives string VR for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
-		}
-		if (trial) octave_stdout << '[' << ov->string_value() << ']' << std::endl ;
-		de->SetByteValue((const char *)ov->string_value().c_str(),len);
-	} else if ( entry.GetVR() & gdcm::VR::US) { // unsigned short
-		if (!ov->is_scalar_type()) { // dicominfo turns US into double, this turns it back to uint16
-			warning(QUOTED(OCT_FN_NAME)": dicomdict gives VR of US for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
-		}
-		if (trial) octave_stdout << '[' << ov->uint16_scalar_value() << ']' << std::endl ;
-		de->SetByteValue((const char *)ov->uint16_array_value().fortran_vec(), gdcm::VL((uint32_t)2));
-	} else if ( entry.GetVR() & gdcm::VR::OB) { // other byte
-		uint8NDArray obv = ov->uint8_array_value();
-		//if (!ov->is_scalar_type()) { // dicominfo seems to return string
-		//	warning(QUOTED(OCT_FN_NAME)": dicomdict gives VR of OB for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
-		//}
-		if (trial) { // TODO surely there is a better way to do this with c++ output stream
-			octave_uint8 * obv_p=obv.fortran_vec();
-			char buf[8];
-			octave_stdout  << '[' ;
-			for (octave_idx_type i=0; i<(octave_idx_type)len; i++) {
-				snprintf(buf,7,"%02X ",(const uint8_t)obv_p[i]);
-				octave_stdout  << buf << " " ;
-			}
-			octave_stdout  << ']' << std::endl ;
-		}
-		de->SetByteValue((const char *)obv.fortran_vec(), gdcm::VL((uint32_t)obv.byte_size()));
-	} else if ( entry.GetVR() & gdcm::VR::DS) { // double string. sep by '\\'
-		if (!ov->is_double_type()) { 
-			warning(QUOTED(OCT_FN_NAME)": dicomdict gives VR of DS for %s, expecting double, octave value is %s", keyword.c_str(), ov->class_name().c_str());
-		}
-		std::stringstream ss;
-		Matrix mat=ov->matrix_value() ; //to matrix of doubles
-		double * mat_p=mat.fortran_vec();
-		for(int i=0; i<mat.numel() ;i++) ss << mat_p[i] << '\\' ;
-		std::string s=ss.str();
-		s=s.substr(0,s.length()-1); // strip last '\\'
-		if (s.length()%2==1) s=s+" "; // ensure even number of chars
-		if (trial) octave_stdout << '[' << s << ']' << std::endl ;
-		de->SetByteValue( (const char *)s.c_str(), gdcm::VL((uint32_t)s.length()) );
-	} else if ( entry.GetVR() & gdcm::VR::IS) { // integer string. only single values, I think
-		if (!ov->is_scalar_type()) { 
-			warning(QUOTED(OCT_FN_NAME)": dicomdict gives VR of IS for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
-		}
-		int32_t val=ov->int32_scalar_value() ;
-		char buf[16];
-		snprintf(buf,14,"%i",val);
-		int len=strlen(buf);
-		if (len%2==1) {buf[len]=' '; buf[len+1]='\0'; }// ensure even number of chars
-		if (trial) octave_stdout << '[' << buf << ']' << std::endl;
-		de->SetByteValue( buf, gdcm::VL((uint32_t)strlen(buf)) );
-	} else if ( entry.GetVR() & gdcm::VR::SQ) { // sequence
-		if (!ov->OV_ISMAP()) { 
-			warning(QUOTED(OCT_FN_NAME)": dicomdict gives VR of SQ for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
-		}
-		octave_stdout << std::endl;
-		//int nObj = ov->numel() ;
-		octave_map subom = ov->map_value();
-		gdcm::SmartPointer<gdcm::SequenceOfItems> sq = new gdcm::SequenceOfItems();
-		try {
-			structarray2sequence(*sq, &subom, trial, ++sequenceDepth) ;
-		} catch (std::exception&) {
-			return;
-		}
-		sequenceDepth--;
-	} else {
-		if (trial) octave_stdout << " ### not handled ### " << std::endl;
-		*handled = false;
-	}
+void structarray2sequence(gdcm::SequenceOfItems & sq, octave_map * om, bool trial, int sequenceDepth)
+{
+  for (octave_map::iterator it = om->begin(); it != om->end(); it++)
+    {
+      gdcm::Item item;
+      // item.SetVLToUndefined(); //TODO: does VL need to be set for items that contain datasets?
+      gdcm::DataSet &nds = item.GetNestedDataSet();
+      std::string itemname(om->key(it));
+      // TODO: test itemname is something like Item_n.
+      Cell cell = om->contents(it);
+      octave_map subom = cell(0).map_value();
+      // octave_stdout << itemname <<std::endl;
+      for (octave_map::iterator subit = subom.begin(); subit != subom.end(); subit++)
+        {
+          std::string subkeyword(subom.key(subit));
+          gdcm::DataElement de;
+          gdcm::Tag tag;
+          bool handled;
+          try
+            {
+              value2element(&de, &(subom.contents(subit)(0)), &tag, subkeyword, trial, &handled, sequenceDepth);
+            }
+          catch (std::exception&)
+            {
+              return;
+            }
+          if (!handled)
+            {
+              warning (QUOTED(OCT_FN_NAME)": element in sequence not handled %s", subkeyword.c_str());
+              continue;
+            }
+          nds.Insert(de); //insert element into dataset
+        }
+      sq.AddItem(item); // add dataset to sequence
+    }
+  return;
 }
 
-// TODO set HighBit etc using octave class. or cast pixel data using metadata, or just give error if they don't agree
-void octaveVal2dicomImage(gdcm::ImageWriter *w, octave_value *pixval) {
-	if (pixval->ndims() != 2) {
-		error(QUOTED(OCT_FN_NAME)": image has %i dimensions. not implemented. ", (int)pixval->ndims());
-		throw std::exception();
-	}
-
-        // get current properties we may have gotten from the input 
-	gdcm::DataSet ds = w->GetFile().GetDataSet();
-	
-	// make image
-	gdcm::SmartPointer<gdcm::Image> im = new gdcm::Image;
-
-	im->SetNumberOfDimensions( 2 );
-	im->SetDimension(0, pixval->dims()(0) );
-	im->SetDimension(1, pixval->dims()(1) );
-
-	gdcm::Attribute<0x0028,0x0004> pi_at;
-	if (ds.FindDataElement(pi_at.GetTag())) {
-	  pi_at.SetFromDataElement( ds.GetDataElement(pi_at.GetTag()) );
-	   
-	  gdcm::PhotometricInterpretation::PIType type = gdcm::PhotometricInterpretation::GetPIType(pi_at.GetValue());
-	  if (type != gdcm::PhotometricInterpretation().GetType())
-	    im->SetPhotometricInterpretation( type );
-	  else
-	    im->SetPhotometricInterpretation( gdcm::PhotometricInterpretation::MONOCHROME1 );
+void value2element (gdcm::DataElement * de, const octave_value * ov, gdcm::Tag * tag, const std::string & keyword, bool trial, bool * handled, int sequenceDepth)
+{
+  gdcm::DictEntry entry;
+  if (!dicom_is_present(keyword))
+    {
+      if (trial)
+        {
+          octave_stdout << std::setw(2*sequenceDepth) << "" << std::setw(0);
+          octave_stdout << keyword << ": not in dictionary" << std::endl;
         }
-	else {
-	  im->SetPhotometricInterpretation( gdcm::PhotometricInterpretation::MONOCHROME1 );
+      *handled = false;
+      return;
+    }
+  lookup_dicom_tag (*tag, keyword);
+  lookup_dicom_entry (entry, *tag);
+  gdcm::VL len((uint32_t)ov->byte_size());
+  //gdcm::DataElement de(*tag, len, entry.GetVR()); 
+  de->SetTag (*tag);
+  de->SetVL (len);
+  de->SetVR (entry.GetVR());
+  *handled = true;
+  if (trial)
+    {
+      octave_stdout << std::setw(2*sequenceDepth) << "" << std::setw(0);
+      octave_stdout << *tag << ':' << entry.GetVR() << ':' << keyword << ':';
+    }
+  if (entry.GetVR() & VRSTRING)
+    {
+      // TODO: check even padding requirement
+      // TODO some dicom string types are stored as numbers
+      if (!ov->is_string())
+        {
+          warning (QUOTED(OCT_FN_NAME)": dicomdict gives string VR for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
         }
-
-	// prepare to make data from mat available
-	char * matbuf = 0; // to be set to point to octave matrix 
-	
-	if (pixval->is_uint8_type()) {
-		uint8NDArray data = pixval->uint8_array_value().transpose();
-                uint8_t * buf = new uint8_t[data.numel ()];
-         	memcpy(buf, data.fortran_vec(), data.numel()*sizeof(uint8_t));
-		matbuf=(char * )buf;
-		im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::UINT8);
-	} else if (pixval->is_uint16_type()) {
-		uint16NDArray data = pixval->uint16_array_value().transpose();
-                uint16_t * buf = new uint16_t[data.numel ()];
-         	memcpy(buf, data.fortran_vec(), data.numel()*sizeof(uint16_t));
-		matbuf=(char * )buf;
-		im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::UINT16);
-	} else if (pixval->is_uint32_type()) {
-		uint32NDArray data = pixval->uint32_array_value().transpose();
-                uint32_t * buf = new uint32_t[data.numel ()];
-         	memcpy(buf, data.fortran_vec(), data.numel()*sizeof(uint32_t));
-		matbuf=(char * )buf;
-		im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::UINT32);
-	} else if (pixval->is_int8_type()) {
-		int8NDArray data = pixval->int8_array_value().transpose();
-                int8_t * buf = new int8_t[data.numel ()];
-         	memcpy(buf, data.fortran_vec(), data.numel()*sizeof(int8_t));
-		matbuf=(char * )buf;
-		im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::INT8);
-	} else if (pixval->is_int16_type()) {
-		int16NDArray data = pixval->int16_array_value().transpose();
-                int16_t * buf = new int16_t[data.numel ()];
-         	memcpy(buf, data.fortran_vec(), data.numel()*sizeof(int16_t));
-		matbuf=(char * )buf;
-		im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::INT16);
-	} else if (pixval->is_int32_type()) {
-		int32NDArray data = pixval->int32_array_value().transpose();
-                int32_t * buf = new int32_t[data.numel ()];
-         	memcpy(buf, data.fortran_vec(), data.numel()*sizeof(int32_t));
-		im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::INT32);
-	} else { // TODO: gdcm::PixelFormat has float types too. 
-		error(QUOTED(OCT_FN_NAME)": cannot write this type"); 
-		throw std::exception();
-	}
-	
-	unsigned long buflen=im->GetBufferLength();
-	if (buflen != pixval->byte_size()) { 
-		delete [] matbuf;
-		error(QUOTED(OCT_FN_NAME)": prepared DICOM buffer size(%lu) does not match Octave array buffer size(%i).",buflen, (int)pixval->byte_size());
-		throw std::exception() ;
-	}
-
-	gdcm::DataElement pixeldata( gdcm::Tag(0x7fe0,0x0010) );
-	pixeldata.SetByteValue( matbuf, buflen );
-
-	im->SetDataElement( pixeldata );
-
-	delete [] matbuf;
-	
-	w->SetImage( *im );
-
-	// set any image values we already have in our data attributes
-	gdcm::Attribute<0x0028,0x0030> sp_at;
-	if (ds.FindDataElement(sp_at.GetTag())) {
-	  sp_at.SetFromDataElement( ds.GetDataElement(sp_at.GetTag()) );
-	  im->SetSpacing(0, sp_at.GetValue(0));
-	  im->SetSpacing(1, sp_at.GetValue(1));
+      if (trial)
+        octave_stdout << '[' << ov->string_value() << ']' << std::endl ;
+      de->SetByteValue ((const char *)ov->string_value().c_str(),len);
+    }
+  else if (entry.GetVR() & gdcm::VR::US)
+    { 
+      // unsigned short
+      if (!ov->is_scalar_type())
+        { 
+          // dicominfo turns US into double, this turns it back to uint16
+          warning (QUOTED(OCT_FN_NAME)": dicomdict gives VR of US for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
+        }
+      if (trial)
+        octave_stdout << '[' << ov->uint16_scalar_value() << ']' << std::endl ;
+      de->SetByteValue ((const char *)ov->uint16_array_value().fortran_vec(), gdcm::VL((uint32_t)2));
+    }
+  else if ( entry.GetVR() & gdcm::VR::OB)
+    { 
+      // other byte
+      uint8NDArray obv = ov->uint8_array_value();
+      //if (!ov->is_scalar_type())
+      //  { 
+      //    // dicominfo seems to return string
+      //    warning (QUOTED(OCT_FN_NAME)": dicomdict gives VR of OB for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
+      //  }
+      if (trial)
+        { 
+          // TODO surely there is a better way to do this with c++ output stream
+          octave_uint8 * obv_p=obv.fortran_vec();
+          char buf[8];
+          octave_stdout  << '[' ;
+          for (octave_idx_type i = 0; i < (octave_idx_type)len; i++)
+            {
+              snprintf (buf, 7, "%02X ", (const uint8_t)obv_p[i]);
+              octave_stdout  << buf << " " ;
+            }
+          octave_stdout  << ']' << std::endl ;
         }
-	gdcm::Attribute<0x0020,0x0037> dir_at;
-	if (ds.FindDataElement(dir_at.GetTag())) {
-	  dir_at.SetFromDataElement( ds.GetDataElement(dir_at.GetTag()) );
-	  im->SetDirectionCosines(0, (double)dir_at.GetValue(0));
-	  im->SetDirectionCosines(1, (double)dir_at.GetValue(1));
-	  im->SetDirectionCosines(2, (double)dir_at.GetValue(2));
-	  im->SetDirectionCosines(3, (double)dir_at.GetValue(3));
-	  im->SetDirectionCosines(4, (double)dir_at.GetValue(4));
-	  im->SetDirectionCosines(5, (double)dir_at.GetValue(5));
+      de->SetByteValue((const char *)obv.fortran_vec(), gdcm::VL((uint32_t)obv.byte_size()));
+    }
+  else if (entry.GetVR() & gdcm::VR::DS)
+    { 
+      // double string. sep by '\\'
+      if (!ov->is_double_type())
+        { 
+          warning (QUOTED(OCT_FN_NAME)": dicomdict gives VR of DS for %s, expecting double, octave value is %s", keyword.c_str(), ov->class_name().c_str());
         }
-	gdcm::Attribute<0x0020,0x0032> or_at;
-	if (ds.FindDataElement(or_at.GetTag())) {
-	  or_at.SetFromDataElement( ds.GetDataElement(or_at.GetTag()) );
-	  im->SetOrigin(0, or_at.GetValue(0));
-	  im->SetOrigin(1, or_at.GetValue(1));
-	  im->SetOrigin(2, or_at.GetValue(2));
+      std::stringstream ss;
+      Matrix mat=ov->matrix_value() ; //to matrix of doubles
+      double * mat_p=mat.fortran_vec();
+      for (int i = 0; i < mat.numel() ;i++)
+        ss << mat_p[i] << '\\' ;
+      std::string s = ss.str();
+      s = s.substr(0,s.length()-1); // strip last '\\'
+      if (s.length()%2==1)
+        s = s +" "; // ensure even number of chars
+      if (trial)
+        octave_stdout << '[' << s << ']' << std::endl ;
+      de->SetByteValue ( (const char *)s.c_str(), gdcm::VL((uint32_t)s.length()) );
+    }
+  else if ( entry.GetVR() & gdcm::VR::IS)
+    { 
+      // integer string. only single values, I think
+      if (!ov->is_scalar_type())
+        { 
+        warning(QUOTED(OCT_FN_NAME)": dicomdict gives VR of IS for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
         }
-	gdcm::Attribute<0x0028,0x1053> sl_at;
-	if (ds.FindDataElement(sl_at.GetTag())) {
-	  sl_at.SetFromDataElement( ds.GetDataElement(sl_at.GetTag()) );
-	  im->SetSlope(sl_at.GetValue());
+      int32_t val=ov->int32_scalar_value() ;
+      char buf[16];
+      snprintf (buf, 14, "%i", val);
+      int len = strlen(buf);
+      // ensure even number of chars
+      if (len%2 == 1) 
+        {
+          buf[len]=' ';
+          buf[len+1]='\0';
         }
-	gdcm::Attribute<0x0028,0x1052> int_at;
-	if (ds.FindDataElement(int_at.GetTag())) {
-	  int_at.SetFromDataElement( ds.GetDataElement(int_at.GetTag()) );
-	  im->SetIntercept(int_at.GetValue());
+      if (trial)
+        octave_stdout << '[' << buf << ']' << std::endl;
+      de->SetByteValue ( buf, gdcm::VL((uint32_t)strlen(buf)) );
+    }
+  else if ( entry.GetVR() & gdcm::VR::SQ)
+    { 
+      // sequence
+      if (!ov->OV_ISMAP())
+        { 
+          warning (QUOTED(OCT_FN_NAME)": dicomdict gives VR of SQ for %s, octave value is %s", keyword.c_str(), ov->class_name().c_str());
         }
+      octave_stdout << std::endl;
+      //int nObj = ov->numel();
+      octave_map subom = ov->map_value();
+      gdcm::SmartPointer<gdcm::SequenceOfItems> sq = new gdcm::SequenceOfItems();
+      try
+        {
+          structarray2sequence(*sq, &subom, trial, ++sequenceDepth) ;
+        }
+      catch (std::exception&)
+        {
+          return;
+        }
+      sequenceDepth--;
+    }
+  else
+    {
+      if (trial)
+        octave_stdout << " ### not handled ### " << std::endl;
+      *handled = false;
+    }
+}
 
-	return ;
+// TODO set HighBit etc using octave class. or cast pixel data using metadata, or just give error if they don't agree
+void octaveVal2dicomImage(gdcm::ImageWriter *w, octave_value *pixval)
+{
+  if (pixval->ndims() != 2)
+    {
+      error (QUOTED(OCT_FN_NAME)": image has %i dimensions. not implemented. ", (int)pixval->ndims());
+      throw std::exception ();
+    }
+
+  // get current properties we may have gotten from the input 
+  gdcm::DataSet ds = w->GetFile().GetDataSet();
+  
+  // make image
+  gdcm::SmartPointer<gdcm::Image> im = new gdcm::Image;
+
+  im->SetNumberOfDimensions (2);
+  im->SetDimension (0, pixval->dims()(0));
+  im->SetDimension (1, pixval->dims()(1));
+
+  gdcm::Attribute<0x0028,0x0004> pi_at;
+  if (ds.FindDataElement (pi_at.GetTag()))
+    {
+      pi_at.SetFromDataElement (ds.GetDataElement (pi_at.GetTag()));
+     
+      gdcm::PhotometricInterpretation::PIType type = gdcm::PhotometricInterpretation::GetPIType(pi_at.GetValue());
+      if (type != gdcm::PhotometricInterpretation().GetType())
+        im->SetPhotometricInterpretation( type );
+      else
+        im->SetPhotometricInterpretation( gdcm::PhotometricInterpretation::MONOCHROME1 );
+    }
+  else
+    {
+      im->SetPhotometricInterpretation( gdcm::PhotometricInterpretation::MONOCHROME1 );
+    }
+
+  // prepare to make data from mat available
+  char * matbuf = 0; // to be set to point to octave matrix 
+  
+  if (pixval->is_uint8_type())
+    {
+      uint8NDArray data = pixval->uint8_array_value().transpose();
+      uint8_t * buf = new uint8_t[data.numel ()];
+      memcpy (buf, data.fortran_vec(), data.numel()*sizeof(uint8_t));
+      matbuf = (char *)buf;
+      im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::UINT8);
+    }
+  else if (pixval->is_uint16_type()) 
+    {
+      uint16NDArray data = pixval->uint16_array_value().transpose();
+      uint16_t * buf = new uint16_t[data.numel ()];
+      memcpy (buf, data.fortran_vec(), data.numel()*sizeof(uint16_t));
+      matbuf = (char *)buf;
+      im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::UINT16);
+    }
+  else if (pixval->is_uint32_type())
+    {
+      uint32NDArray data = pixval->uint32_array_value().transpose();
+      uint32_t * buf = new uint32_t[data.numel ()];
+      memcpy (buf, data.fortran_vec(), data.numel()*sizeof(uint32_t));
+      matbuf = (char *)buf;
+      im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::UINT32);
+    }
+  else if (pixval->is_int8_type())
+    {
+      int8NDArray data = pixval->int8_array_value().transpose();
+      int8_t * buf = new int8_t[data.numel ()];
+      memcpy (buf, data.fortran_vec(), data.numel()*sizeof(int8_t));
+      matbuf = (char *)buf;
+      im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::INT8);
+    }
+  else if (pixval->is_int16_type())
+    {
+      int16NDArray data = pixval->int16_array_value().transpose();
+      int16_t * buf = new int16_t[data.numel ()];
+      memcpy (buf, data.fortran_vec(), data.numel()*sizeof(int16_t));
+      matbuf = (char *)buf;
+      im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::INT16);
+    }
+  else if (pixval->is_int32_type())
+    {
+      int32NDArray data = pixval->int32_array_value().transpose();
+      int32_t * buf = new int32_t[data.numel ()];
+      memcpy (buf, data.fortran_vec(), data.numel()*sizeof(int32_t));
+      matbuf = (char *)buf;
+      im->GetPixelFormat().SetScalarType(gdcm::PixelFormat::INT32);
+    }
+  else 
+    { // TODO: gdcm::PixelFormat has float types too. 
+      error(QUOTED(OCT_FN_NAME)": cannot write this type"); 
+      throw std::exception();
+    }
+  
+  unsigned long buflen = im->GetBufferLength();
+  if (buflen != pixval->byte_size())
+    { 
+      delete [] matbuf;
+      error (QUOTED(OCT_FN_NAME)": prepared DICOM buffer size(%lu) does not match Octave array buffer size(%i).",buflen, (int)pixval->byte_size());
+      throw std::exception();
+    }
+
+  gdcm::DataElement pixeldata (gdcm::Tag(0x7fe0,0x0010));
+  pixeldata.SetByteValue (matbuf, buflen);
+
+  im->SetDataElement (pixeldata);
+
+  delete [] matbuf;
+  
+  w->SetImage (*im);
+
+  // set any image values we already have in our data attributes
+  gdcm::Attribute<0x0028,0x0030> sp_at;
+  if (ds.FindDataElement(sp_at.GetTag()))
+    {
+      sp_at.SetFromDataElement( ds.GetDataElement(sp_at.GetTag()) );
+      im->SetSpacing(0, sp_at.GetValue(0));
+      im->SetSpacing(1, sp_at.GetValue(1));
+    }
+  gdcm::Attribute<0x0020,0x0037> dir_at;
+  if (ds.FindDataElement(dir_at.GetTag()))
+    {
+      dir_at.SetFromDataElement( ds.GetDataElement(dir_at.GetTag()) );
+      im->SetDirectionCosines(0, (double)dir_at.GetValue(0));
+      im->SetDirectionCosines(1, (double)dir_at.GetValue(1));
+      im->SetDirectionCosines(2, (double)dir_at.GetValue(2));
+      im->SetDirectionCosines(3, (double)dir_at.GetValue(3));
+      im->SetDirectionCosines(4, (double)dir_at.GetValue(4));
+      im->SetDirectionCosines(5, (double)dir_at.GetValue(5));
+    }
+  gdcm::Attribute<0x0020,0x0032> or_at;
+  if (ds.FindDataElement(or_at.GetTag()))
+    {
+      or_at.SetFromDataElement( ds.GetDataElement(or_at.GetTag()) );
+      im->SetOrigin(0, or_at.GetValue(0));
+      im->SetOrigin(1, or_at.GetValue(1));
+      im->SetOrigin(2, or_at.GetValue(2));
+    }
+  gdcm::Attribute<0x0028,0x1053> sl_at;
+  if (ds.FindDataElement(sl_at.GetTag()))
+    {
+      sl_at.SetFromDataElement( ds.GetDataElement(sl_at.GetTag()) );
+      im->SetSlope(sl_at.GetValue());
+    }
+  gdcm::Attribute<0x0028,0x1052> int_at;
+  if (ds.FindDataElement(int_at.GetTag()))
+    {
+      int_at.SetFromDataElement( ds.GetDataElement(int_at.GetTag()) );
+      im->SetIntercept(int_at.GetValue());
+    }
+
+  return;
 }
 
-void genMinimalMetaData(gdcm::ImageWriter *w, gdcm::File *file){
-	gdcm::UIDGenerator uid; // helper for uid generation
-
-	// Step 2: DERIVED object
-	gdcm::FileDerivation fd; //TODO: copied this: don't understand it
-	// For the pupose of this execise we will pretend that this image is referencing
-	// two source image (we need to generate fake UID for that).
-	const char ReferencedSOPClassUID[] = "1.2.840.10008.5.1.4.1.1.7"; // Secondary Capture
-	fd.AddReference( ReferencedSOPClassUID, uid.Generate() );
-	fd.AddReference( ReferencedSOPClassUID, uid.Generate() );
-
-	// Again for the purpose of the exercise we will pretend that the image is a
-	// multiplanar reformat (MPR):
-	// CID 7202 Source Image Purposes of Reference
-	// {"DCM",121322,"Source image for image processing operation"},
-	fd.SetPurposeOfReferenceCodeSequenceCodeValue( 121322 );
-	// CID 7203 Image Derivation
-	// { "DCM",113072,"Multiplanar reformatting" },
-	fd.SetDerivationCodeSequenceCodeValue( 113072 );
-	fd.SetFile( *file );
-	// If all Code Value are ok the filter will execute properly
-	if( !fd.Derive() ) {
-		error(QUOTED(OCT_FN_NAME)": file derivation failed");
-		throw std::exception();
-	}
-	
-	w->SetFile( fd.GetFile() );
-
-	return ;
+void genMinimalMetaData(gdcm::ImageWriter *w, gdcm::File *file)
+{
+  gdcm::UIDGenerator uid; // helper for uid generation
+
+  // Step 2: DERIVED object
+  gdcm::FileDerivation fd; //TODO: copied this: don't understand it
+  // For the pupose of this execise we will pretend that this image is referencing
+  // two source image (we need to generate fake UID for that).
+  const char ReferencedSOPClassUID[] = "1.2.840.10008.5.1.4.1.1.7"; // Secondary Capture
+  fd.AddReference( ReferencedSOPClassUID, uid.Generate() );
+  fd.AddReference( ReferencedSOPClassUID, uid.Generate() );
+
+  // Again for the purpose of the exercise we will pretend that the image is a
+  // multiplanar reformat (MPR):
+  // CID 7202 Source Image Purposes of Reference
+  // {"DCM",121322,"Source image for image processing operation"},
+  fd.SetPurposeOfReferenceCodeSequenceCodeValue( 121322 );
+  // CID 7203 Image Derivation
+  // { "DCM",113072,"Multiplanar reformatting" },
+  fd.SetDerivationCodeSequenceCodeValue( 113072 );
+  fd.SetFile( *file );
+  // If all Code Value are ok the filter will execute properly
+  if (! fd.Derive())
+    {
+      error(QUOTED(OCT_FN_NAME)": file derivation failed");
+      throw std::exception();
+    }
+  
+  w->SetFile (fd.GetFile ());
+
+  return;
 }
 /*
 %!shared testfile1, testfile2