bugGNU Octave - Bugs: bug #62910, [octave forge] (image)...

 
 

bug #62910: [octave forge] (image) bwslect(...,4) in Octave behaves like bwslect(...,8) in MATLAB and vice versa

Submitter:  Avinoam Kalma <avinoam>
Submitted:  Thu 18 Aug 2022 05:00:19 AM UTC
 
Category:  Octave Forge Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Ready For Test Assigned to:  avinoam
Originator Name:  Avinoam Open/Closed:  * Open
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

Wed 28 Sep 2022 04:00:53 PM UTC, comment #4: 

I have pushed a fix:
http://hg.code.sf.net/p/octave/image/rev/2aa03116d219

including the remarks from comment #2.

To improve MATLAB compatibility I have added

if (nargout == 0)
     figure; imshow(imout);
endif

Please review. Thanks

Avinoam Kalma <avinoam>
Project Member
Sat 24 Sep 2022 11:31:42 AM UTC, comment #3: 

Thanks, Hartmut for reviewing and for the valuable comments.
I will push an updated fix.

Avinoam Kalma <avinoam>
Project Member
Sat 24 Sep 2022 10:31:44 AM UTC, comment #2: 

Thanks for finding and fixing this, Avinoam!

I have reviewed your patch in comment #1:

  • It fixes the bug from the demo code in comment #0
  • It adds a couple of unit tests to bwselect.m. They all pass.
  • All new unit tests are Matlab compatible in their results. (but see below)
  • All unit tests in bwfill also still pass after this change in bwselect.

I would suggest some small improvements before pushing the patch to the repo:

  • In Matlab a call like "bwselect (A, 3, 3)" returns an image handle and no number array. To improve the Matlab compatibility of the new unit tests, you could change this to "out = bwselect (A, 3, 3)" and then use the "out" value in the call to assert.
  • There are some missing spaces after the function names "zeros" and "logical" in the new unit tests.
  • Please announce the changed behavior of bwselect in the NEWS file.

Since this patch does change the function behavior, in my opinion it should go the the default branch of the image repository.

Hartmut <hardy>
Thu 18 Aug 2022 05:48:29 AM UTC, comment #1: 

Please review the attached patch.
Should it be in the stable branch or in the default branch?

(file #53579)

Avinoam Kalma <avinoam>
Project Member
Thu 18 Aug 2022 05:00:19 AM UTC, original submission:  

Bug #50071 was "[octave forge] (image) bwfill(...,4) in Octave behaves like bwfill(...,8) in MATLAB and vice versa".
This bug has been fixed.

But bwslect calls bwfill, and this fix was not good for bwslect.

For example, in MATLAB

>> img = [0,1,0;0,1,1;1,0,1]; A = bwselect(~img,1,1,4)
ans =

   1   0   0
   1   0   0
   0   0   0

>> img = [0,1,0;0,1,1;1,0,1]; A = bwselect(~img,1,1,8)
ans =

   1   0   0
   1   0   0
   0   1   0

In Octave the results are the opposite.
I will prepare a patch to fix this

Avinoam Kalma <avinoam>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #53579:  bwselect.diff added by avinoam (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by avinoam (Submitted the item)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by avinoam
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only project members can vote.

     

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-09-28 avinoam StatusPatch Reviewed Ready For Test
    2022-09-24 avinoam StatusNone Patch Reviewed
    2022-08-18 avinoam Attached File- Added bwselect.diff, #53579
    2022-08-18 avinoam Carbon-Copy- Added carandraug
        Carbon-Copy- Added hardy

    Back to the top


    Powered by Savane 3.9