[Octave-patch-tracker] [patch #8612] impyramid.m for image package

[Carnë Draug]

Follow-up Comment #1, patch #8612 (project octave):

Some comments:

1. the results are wrong for the image border. I will guess that the problem
is the values used on the border during the convolution.  Seems like Matlab
2010b will have the following results (using your tests)


reduced =

  114  139  131  103  111
   97  122  141  111  100
  103  123  112  123  122
   47  107  134  153   94



expanded =

  115  154  185  178  150  122  105  116  138  159  158  117   78   86  112 
129  133  120  103
   69   98  128  141  146  152  152  139  125  127  121   87   55   58   81 
113  131  112   84
   40   54   74  100  131  167  184  157  119  104   92   64   41   44   66 
100  121  103   74
   76   69   65   75   97  130  153  148  131  122  108   80   61   79  103 
105   98   97   98
  120  105   88   77   78   96  121  143  155  154  140  112   98  124  143 
109   74   91  123
  117  129  134  119  107  125  153  173  180  172  156  143  138  146  140  
96   60   83  122
   99  139  170  157  139  156  181  188  180  164  151  154  156  140  112  
81   65   84  110
  101  136  163  153  133  132  138  136  130  122  120  130  133  108   82  
86   99  104  104
  103  126  143  136  116   97   81   73   73   82   94  105  105   87   78 
108  138  133  116
   90  116  139  139  122   96   69   52   53   80  109  114  111  116  128 
148  163  164  160
   66   99  131  140  131  109   83   62   62  102  142  144  138  154  169 
164  157  169  184
   41   68   99  121  130  122  107   92   95  133  173  182  172  156  135 
114  105  121  142
   21   38   64   98  124  131  127  123  129  160  194  212  199  144   82  
52   48   65   85


2. You are setting the direction default to reduce.  A default makes sense
when there's a value that makes more sense than the others (sometimes even
changing depending on the rest of the input).  This is not the case. Could you
drop the default value and require 2 nargin?

3. isgray and isrgb will check the actual numeric values. If the input is
class double, it will return false for values outside [0 1]. Please only check
if input isnumeric.

4. like the point above, this should work for n dimensional input and not be
limited to number of dimensions (think that input may be of size [512, 789,
12, 23] and we should not be questioning that). At the moment, isrgb and
isgray would fail if 3rd dimension is not 1 or 3 which would prevent this. The
next weakest link would be imfilter which would not handle such thing. Could
you fix imfilter to handle this?

The main trick to handle this type of things is to get a vector of dimensions,
use them to create a cell array for the indices, and then turn it into a comma
separated list (cell{:}) for indexing, like in


s = [16 16 16]
idx = arrayfun (@colon, ones (1, 3), s, "UniformOutput", false)
img(idx{:}) ...


5. you do not need to check ischar (direction). Simply use on the switch
block. If it does not match anything, then you can use the otherwise to give a
better error message (either is not a string or an invalid string value

6. please no hard tabs, only spaces. Also, could you please remove the
trailing whitespace?

7. could you add your name and email to the copyright?

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8612>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/