octave-patch-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-patch-tracker] [patch #9354] image package: new function wiener2


From: Carnë Draug
Subject: [Octave-patch-tracker] [patch #9354] image package: new function wiener2.m
Date: Fri, 26 May 2017 21:06:11 -0400 (EDT)
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0

Follow-up Comment #7, patch #9354 (project octave):

Four comments:

1. instead of checking if it's of class double, I think it's better to just
use isfloat.  That will prevent the conversion to double if input is of class
single but that is the whole point of it. If someone uses single instead of
double they already made the choice of paying precision to save memory.

2. just use 'mean (x(:))' instead of 'mean2 (x)' which is clearer, and returns
the same. mean2 is really only for sake of matlab compatibility.

3. if the use of imfilter was replaced with padarray and convn, is there
anything preventing it from working with nd dimensions? (despite the name
wiener2)

4. at the end you could return mean_im and do 'mean_im += ...' instead of
'im_out = mean_im + ...' which performs in place addition and have one less
variable the size of the image in memory.

Other than that, there's only a few style changes which I can fix myself like
3 space indentation and trailing whitespace.

A minor note on the use of mercurial: you made use of a branch named
new-wiener2 but a bookmark was better.  Mercurial bookmarks are akin to git
branches.  Mercurial branches are something else without an equivalent on git.
 Mercurial branches are meant for long term development, such as stable and
development branches.  For short things like development a new feature, an
extra head on an existing branch would be better suited, and a bookmark just
puts a name to that extra head.

    _______________________________________________________

Reply to this item at:

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

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]