Re: ismember bugs

[Søren Hauberg]

Søren Hauberg skrev:
> Sorry about replying to my own post, but I used part of my just-posted 
> code to patch the current implementation of ismember. I still don't 
> understand that implementation, but I can fix the bugs (patch attached). 
> This assumes that the cellstr bug is fixed.
Once again I'm replying to my own post. I'm beginning to feel like a 
spammer...
Anyway, attached is a patch that I think cleans ismember up a bit. It 
also adds support for a second output argument like matlab does. 
Assuming the cellstr bug gets fixed it should pass all tests. I still 
don't really understand the code, so I don't think this patch should be 
applied before somebody else has tested it as well. I did _not_ add 
support for the 'rows' argument that matlab has. I'll leave that as an 
exercise to the reader :-)

Søren

--- /home/sh/Programmer/share/octave/2.9.13/m/set/ismember.m    2007-08-10 
22:48:08.000000000 +0200
+++ ismember.m  2007-08-11 11:34:39.000000000 +0200
@@ -18,59 +18,68 @@
 ## 02110-1301, USA.
 
 ## -*- texinfo -*-
-## @deftypefn {Function File} {} ismember (@var{A}, @var{S})
-## Return a matrix the same shape as @var{A} which has 1 if
-## @code{A(i,j)} is in @var{S} or 0 if it isn't.
+## @deftypefn {Function File} address@hidden, @var{index}] = ismember 
(@var{A}, @var{S})
+## Return a matrix @var{bool} the same shape as @var{A} which has 1 if
+## @code{A(i,j)} is in @var{S} or 0 if it isn't. If a second output argument
+## is requested, the indexes into  @var{S} of the matching elements is
+## also returned.
 ## @seealso{unique, union, intersection, setxor, setdiff}
 ## @end deftypefn
 
 ## Author: Paul Kienzle
 ## Adapted-by: jwe
 
-function c = ismember (a, S)
+function [c, index] = ismember (a, S, rows_opt)
 
-  if (nargin != 2)
+  if (nargin != 2 && nargin != 3)
     print_usage ();
   endif
 
+  ## We currently don't handle the 'rows' argument
+  if (nargin == 3)
+    ## XXX: Should this rather be an error?
+    warning("ismember: 'rows' argument not supported");
+  endif
+
+  ## Convert char matrices to cell arrays
+  if (ischar(a))
+    a = cellstr(a);
+  endif
+  if (ischar(S))
+    S = cellstr(S);
+  endif
+  
+  ## Input checking
+  if ( !isa(a, class(S)) )
+    error("ismember: both input arguments must be the same type");
+  endif
+  if (iscell(a) && !iscellstr(a))
+    error("ismember: cell arrays may only contain strings");
+  endif
+  if (!isnumeric(a) && !iscell(a))
+    error("ismember: input arguments must be arrays, cell arrays, or strings");
+  endif
+  
+  ## Do the actual work
   if (isempty (a) || isempty (S))
     c = zeros (size (a), "logical");
   else
-    if (iscell (a) && ! iscell (S))
-      tmp{1} = S;
-      S = tmp;
-    endif
-    if (! iscell (a) && iscell (S))
-      tmp{1} = a;
-      a = tmp;
-    endif
-    S = unique (S(:));
-    lt = length (S);
-    if (lt == 1)
-      if (iscell (a) || iscell (S))
-        c = cellfun ("length", a) == cellfun ("length", S);
-        idx = find (c);
-       if (isempty (idx))
-         c = zeros (size (a), "logical");
-       else
-         c(idx) = all (char (a(idx)) == repmat (char (S), length (idx), 1), 2);
-       endif
-      else
+    if (numel (S) == 1)
+      if (iscell(a))
+        c = strcmp(a, S);
+      else # 'a' and 'S' are matrices
         c = (a == S);
       endif
+      index = double(c);
     elseif (numel (a) == 1)
-      if (iscell (a) || iscell (S))
-        c = cellfun ("length", a) == cellfun ("length", S);
-        idx = find (c);
-       if (isempty (idx))
-         c = zeros (size (a), "logical");
-       else
-          c(idx) = all (repmat (char (a), length (idx), 1) == char (S(idx)), 
2);
-          c = any(c);
-       endif
-      else
-        c = any (a == S);
+      if (iscell (a))
+        f = find(strcmp(a, S), 1);
+      else # 'a' and 'S' are matrices
+        f = find(a == S, 1);
       endif
+      c = !isempty(f);
+      index = f;
+      if (isempty(index)), index=0; endif
     else
       ## Magic:  the following code determines for each a, the index i
       ## such that S(i)<= a < S(i+1).  It does this by sorting the a
@@ -101,16 +110,22 @@
       ## easy to now check membership by comparing S(a_idx) == a.  This
       ## magic works because S starts out sorted, and because sort
       ## preserves the relative order of identical elements.
+      lt = length(S);
+      [S, Sidx] = sort(S);
       [v, p] = sort ([S(2:lt); a(:)]);
       idx(p) = cumsum (p <= lt-1) + 1;
       idx = idx(lt:end);
-      if (iscell (a) || iscell (S))
+      if (iscell (a))
         c = (cellfun ("length", a)
-            == reshape (cellfun ("length", S(idx)), size (a)));
+             == reshape (cellfun ("length", S(idx)), size (a)));
         idx2 = find (c);
         c(idx2) = all (char (a(idx2)) == char (S(idx)(idx2)), 2);
-      else
+        index = zeros(size(c));
+        index(c) = Sidx(idx(c));
+      else # 'a' and 'S' are matrices
         c = (a == reshape (S (idx), size (a)));
+        index = zeros(size(c));
+        index(c) = Sidx(idx(c));
       endif
     endif
   endif
@@ -121,7 +136,7 @@
 %!assert (ismember ('abc', {'abc', 'def'}), true);
 %!assert (isempty (ismember ([], [1, 2])), true);
 %!xtest assert (ismember ('', {'abc', 'def'}), false);
-%!xtest fail ('ismember ([], {1, 2})', 'error:.*');
+%!fail ('ismember ([], {1, 2})', 'error:.*');
 %!fail ('ismember ({[]}, {1, 2})', 'error:.*');
 %!assert (ismember ({'foo', 'bar'}, {'foobar'}), logical ([0, 0]))
 %!assert (ismember ({'foo'}, {'foobar'}), false)