[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/
- [Octave-patch-tracker] [patch #9730] [octave forge] (image) new function imfuse,
Hartmut <=