changeset 21742:c86cacc3aaf4

imread: Clean up function and its private implementation. * imread.m: Improve docstring. * __imread__.m: Remove redundant input validation done in imread.m. Recode input validation for Index option. Remove stray commas on case statements.
author Rik <rik@octave.org>
date Thu, 19 May 2016 08:42:55 -0700
parents 71d8a9a0642c
children f4d7d0eb5b0c
files scripts/image/imread.m scripts/image/private/__imread__.m
diffstat 2 files changed, 58 insertions(+), 63 deletions(-) [+]
line wrap: on
line diff
--- a/scripts/image/imread.m	Thu May 19 23:54:09 2016 +1000
+++ b/scripts/image/imread.m	Thu May 19 08:42:55 2016 -0700
@@ -26,60 +26,59 @@
 ## @deftypefnx {} {[@dots{}] =} imread (@var{url})
 ## @deftypefnx {} {[@dots{}] =} imread (@dots{}, @var{ext})
 ## @deftypefnx {} {[@dots{}] =} imread (@dots{}, @var{idx})
-## @deftypefnx {} {[@dots{}] =} imread (@dots{}, @var{param1}, @var{val1}, @dots{})
+## @deftypefnx {} {[@dots{}] =} imread (@dots{}, @var{param1}, @var{value1}, @dots{})
 ## Read images from various file formats.
 ##
-## Read an image as a matrix from the file @var{filename}.  If there is no file
-## @var{filename}, and @var{ext} was specified, it will look for a file with
-## the extension @var{ext}.  Finally, it will attempt to download and read an
-## image from @var{url}.
+## Read an image as a matrix from the file @var{filename} or from the online
+## resource @var{url}.  If neither is given, but @var{ext} was specified, look
+## for a file with the extension @var{ext}.
 ##
 ## The size and class of the output depends on the format of the image.  A
-## color image is returned as an @nospell{MxNx3} matrix.  Gray-level and
-## black-and-white images are of size @nospell{MxN}.  Multipage images will
+## color image is returned as an @nospell{MxNx3} matrix.  Grayscale and
+## black-and-white images are of size @nospell{MxN}@.  Multipage images will
 ## have an additional 4th dimension.
 ##
 ## The bit depth of the image determines the class of the output:
-## @qcode{"uint8"}, @qcode{"uint16"} or @qcode{"single"} for gray and color,
-## and @qcode{"logical"} for black and white.  Note that indexed images always
-## return the indexes for a colormap, independent if @var{map} is a requested
-## output.  To obtain the actual RGB image, use @code{ind2rgb}.  When more
-## than one indexed image is being read, @var{map} is obtained from the
-## first.  In some rare cases this may be incorrect and @code{imfinfo} can be
-## used to obtain the colormap of each image.
+## @qcode{"uint8"}, @qcode{"uint16"}, or @qcode{"single"} for grayscale and
+## color, and @qcode{"logical"} for black-and-white.  Note that indexed images
+## always return the indexes for a colormap, independent of whether @var{map}
+## is a requested output.  To obtain the actual RGB image, use @code{ind2rgb}.
+## When more than one indexed image is being read, @var{map} is obtained from
+## the first.  In some rare cases this may be incorrect and @code{imfinfo} can
+## be used to obtain the colormap of each image.
 ##
 ## See the Octave manual for more information in representing images.
 ##
 ## Some file formats, such as TIFF and GIF, are able to store multiple images
 ## in a single file.  @var{idx} can be a scalar or vector specifying the
-## index of the images to read.  By default, Octave will only read the first
+## index of the images to read.  By default, Octave will read only the first
 ## page.
 ##
 ## Depending on the file format, it is possible to configure the reading of
-## images with @var{param}, @var{val} pairs.  The following options are
+## images with @var{parameter}, @var{value} pairs.  The following options are
 ## supported:
 ##
-## @table @samp
+## @table @asis
 ## @item @qcode{"Frames"} or @qcode{"Index"}
 ## This is an alternative method to specify @var{idx}.  When specifying it
 ## in this way, its value can also be the string @qcode{"all"}.
 ##
 ## @item @qcode{"Info"}
-## This option exists for @sc{matlab} compatibility and has no effect.  For
-## maximum performance while reading multiple images from a single file, use
-## the Index option.
+## This option exists for @sc{matlab} compatibility, but has no effect.  For
+## maximum performance when reading multiple images from a single file, use
+## the @qcode{"Index"} option.
 ##
 ## @item @qcode{"PixelRegion"}
-## Controls the image region that is read.  Takes as value a cell array with
-## two arrays of 3 elements @code{@{@var{rows} @var{cols}@}}.  The elements
-## in the array are the start, increment and end pixel to be read.  If the
-## increment value is omitted, defaults to 1.  For example, the following are
-## all equivalent:
+## Controls the image region that is read.  The value must be a cell array with
+## two arrays of 3 elements @code{@{[@var{rows}], [@var{cols}]@}}.  The
+## elements in the array are the start, increment, and end pixel to be read.
+## If the increment value is omitted it defaults to 1.  For example, the
+## following are all equivalent:
 ##
 ## @example
 ## @group
-## imread (filename, "PixelRegion", @{[200 600] [300 700]@});
-## imread (filename, "PixelRegion", @{[200 1 600] [300 1 700]@});
+## imread (filename, "PixelRegion", @{[200 600], [300 700]@});
+## imread (filename, "PixelRegion", @{[200 1 600], [300 1 700]@});
 ## imread (filename)(200:600, 300:700);
 ## @end group
 ## @end example
@@ -97,6 +96,7 @@
 ## Author: Andy Adler
 
 function [img, varargout] = imread (filename, varargin)
+
   if (nargin < 1)
     print_usage ();
   elseif (! ischar (filename))
@@ -212,3 +212,4 @@
 %! assert (class (r), "uint8");
 %! assert (isempty (cmap));
 %! assert (isempty (a));
+
--- a/scripts/image/private/__imread__.m	Thu May 19 23:54:09 2016 +1000
+++ b/scripts/image/private/__imread__.m	Thu May 19 08:42:55 2016 -0700
@@ -21,9 +21,9 @@
 ## along with Octave; see the file COPYING.  If not, see
 ## <http://www.gnu.org/licenses/>.
 
-## This function does all the work of imread. It exists here as private
+## This function does all the work of imread.  It exists here as private
 ## function so that imread can use other functions if imformats is
-## configured to. It is also needed so that imformats can create a
+## configured to.  It is also needed so that imformats can create a
 ## function handle for it.
 
 ## Author: Carnë Draug <carandraug@octave.org>
@@ -35,70 +35,64 @@
 
 function varargout = __imread__ (filename, varargin)
 
-  if (nargin < 1)
-    print_usage ("imread");
-  elseif (! ischar (filename))
-    error ("imread: FILENAME must be a string");
-  endif
-
   ## keep track of the varargin offset we're looking at each moment
   offset = 1;
 
-  ## It is possible for an file with multiple pages to have very different
-  ## images on each page. Specifically, they may have different sizes. Because
-  ## of this, we need to first find out the index of the images to read so
-  ## we can set up defaults for things such as PixelRegion later on.
+  ## It is possible for a file with multiple pages to have very different
+  ## images on each page.  Specifically, they may have different sizes.
+  ## Because of this, we need to first find out the index of the images to read
+  ## so we can set up defaults for things such as PixelRegion later on.
   options = struct ("index", 1);  # default image index
 
   ## Index is the only option that can be defined without the parameter/value
-  ## pair style. When defining it here, the string "all" is invalid though.
-  ## Also, for matlab compatibility, if index is defined both as an option here
+  ## pair style.  When defined here, the string "all" is invalid.
+  ## Also, for Matlab compatibility, if index is defined both as an option here
   ## and parameter/value pair, silently ignore the first.
-  if (nargin >= offset + 1 && ! ischar (varargin{offset}))
+  if (nargin >= 2 && ! ischar (varargin{1}))
+    options.index = varargin{1};
     if (! is_valid_index_option (options.index))
       error ("imread: IDX must be a numeric vector");
     endif
-    options.index = varargin{offset};
-    offset += 1;
+    offset = 2;
   endif
 
   if (rem (numel (varargin) - offset + 1, 2) != 0)
-    error ("imread: no pair for all arguments (odd number left over)");
+    error ("imread: PARAM/VALUE arguments must occur in pairs");
   endif
 
-  ## Check key/value options.
-  indexes = cellfun ("isclass", varargin, "char");
-  indexes(indexes) &= ismember (tolower (varargin(indexes)),
-                                {"frames", "index"});
-  indexes = find (indexes);
-  if (indexes)
-    options.index = varargin{indexes+1};
-    if (! is_valid_index_option (options.index)
-        && ! (ischar (options.index) && strcmpi (options.index, "all")))
-      error ("imread: value for %s must be a vector or the string `all'");
+  ## Check for Index/Frames argument
+  idx = strcmpi ("index", varargin) | strcmpi ("frames", varargin);
+  if (any (idx))
+    if (sum (idx) > 1)
+      error ("imread: Index or Frames may only be specified once");
     endif
+    val = varargin{shift (idx, 1)};
+    if (! is_valid_index_option (val) && ! strcmpi (val, "all"))
+      error ("imread: %s must be a vector or the string `all'", varargin{idx});
+    endif
+    options.index = val;
   endif
 
   ## Use information from the first image to be read to set defaults.
-  if (ischar (options.index) && strcmpi (options.index, "all"))
+  if (strcmpi (options.index, "all"))
     info = __magick_ping__ (filename, 1);
   else
     info = __magick_ping__ (filename, options.index(1));
   endif
 
   ## Set default for options.
-  options.region = {1:1:info.rows 1:1:info.columns};
+  options.region = {1:1:info.rows, 1:1:info.columns};
 
   for idx = offset:2:(numel (varargin) - offset + 1)
     switch (tolower (varargin{idx}))
 
       case {"frames", "index"}
-        ## Do nothing. This options were already processed before the loop.
+        ## Do nothing.  This option was already processed before the loop.
 
-      case "pixelregion",
+      case "pixelregion"
         options.region = varargin{idx+1};
         if (! iscell (options.region) || numel (options.region) != 2)
-          error ("imread: value for %s must be a 2 element cell array",
+          error ("imread: %s must be a 2-element cell array",
                  varargin{idx});
         endif
         for reg_idx = 1:2
@@ -121,8 +115,8 @@
           error ("imread: end COLS for PixelRegions option is larger than image width");
         endif
 
-      case "info",
-        ## We ignore this option. This parameter exists in Matlab to
+      case "info"
+        ## We ignore this option.  This parameter exists in Matlab to
         ## speed up the reading of multipage TIFF by passing a structure
         ## that contains information about the start on the file of each
         ## page.  We can't control it through GraphicsMagic but at least
@@ -138,7 +132,7 @@
 
 endfunction
 
-## Tests if the value passed to the Index or Frames is valid. This option
+## Test if the value passed to the Index or Frames is valid.  This option
 ## can be defined in two places, but only in one place can it also be the
 ## string "all"
 function bool = is_valid_index_option (arg)