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

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

[Octave-patch-tracker] [patch #9730] [octave forge] (image) new function


From: Hartmut
Subject: [Octave-patch-tracker] [patch #9730] [octave forge] (image) new function imfuse
Date: Sun, 14 Mar 2021 13:16:25 -0400 (EDT)
User-agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:86.0) Gecko/20100101 Firefox/86.0

Follow-up Comment #20, patch #9730 (project octave):

I have now had a closer look into the file imfuse.m that results when applying
all patches from comment #17 (v4). As a conclusion I would be happy to see
this file added to the image repo as it is now.

I have tested the following:
* All examples from the Matlab support page produce (visually) the same
results as Matlab does.
* All current BIST tests pass. (I have not looked into the two failing xtests.
Avinoam seems to have discussed this already.)
* Most of my initial comments for improvement (comment #4) are now fine.
* There are still no BIST tests for more exotic input types (int16, int32,
...) I have done a quick test for myself. Result: Using imfuse on an int16
image together with a int8 image gives an unintuitive result, but this result
looks the same as in Matlab. Using two int16 input images give a visually
useful result, this also looks like the one I get from Matlab. So (at least
for int16) this code seems to work fine for other input types, anyways.

I have seem some minor style issues, that might be (only!) nice to change:
* There a many line breaks in code lines. Some use "..." others use a
backslash. Could be use the same kind of line continuation marks (e.g. always
"...") on all of those?
* I would like to have those line breaks at positions such that the code
remains human readable. E.g. the current line 103 looks weired to me.
* There are a few spaces missing before the opening brackets of a function.
Some of them might well not be fixable. But the one on line 306 is.

@Avinoam. Yes, please go ahead and push this version of imfuse to the image
repo, including the necessary bookkeeping changes. 

@Martin Janda: Thanks a lot for implementing this nice and useful function for
the Octave image package. (And if you should find the time for imshowpair, I
would also be happy to see it!)

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?9730>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/




reply via email to

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