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

 
 

bug #59953: [octave forge] (image) imtransform xdata can't handle descending order

Submitter:  None
Submitted:  Wed 27 Jan 2021 09:30:55 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  avinoam
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * other
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 22 Nov 2021 06:38:25 AM UTC, comment #10: 

Pushed, with Hartmut's corrections (comment #8).
http://hg.code.sf.net/p/octave/image/rev/b997f579dbae

Closing as fixed.
Thanks.

Avinoam Kalma <avinoam>
Group Member
Thu 11 Nov 2021 05:04:56 AM UTC, comment #9: 

Thanks, Hartmut, for the detailed review.
I will make the corrections and push the corrected version.

Avinoam Kalma <avinoam>
Group Member
Wed 10 Nov 2021 04:24:12 PM UTC, comment #8: 

Sorry for the late reply, I have now reviewd the latest patch (V2, the diff file only) from Avinoam:

  • all tests still pass
  • the demo still works
  • the patched imtransform returns the Matlab compatible values for the code mentioned originally (comment #0) in this bug report.
  • there are new tests added for this new functionality
  • the coding guidelines are mostly fulfilled, some things that COULD maybe improved (but this is mostly only taste):
    • add a space sign after each semicolon in the definition of "im" in the new test. Maybe also do this for the manually typed matrices in the final assert statements in the new test.
    • maybe add an empty line after the new line 188, i.e. after the new (and commented) code piece.
  • There are some spelling errors the should probably be fixed before pushing:
    • The mistyped word "vetical" shouls probably read "vertical".
    • in the doc text: the formulation "which cause" should probably read "which causes".


After probably fixing the small typos in the doc strings, I would like to see this patch pushed to the image repo. Thanks for fixing this bug, Avinoam.

Hartmut <hardy>
Wed 06 Oct 2021 04:44:04 AM UTC, comment #7: 

Ping: please review the attached fix.
Thanks

Avinoam Kalma <avinoam>
Group Member
Mon 30 Aug 2021 05:12:29 AM UTC, comment #6: 

udata and vdata should be in ascending order.
Attached revised fix.
Please review

(file #51834, file #51835)

Avinoam Kalma <avinoam>
Group Member
Sun 29 Aug 2021 08:50:47 PM UTC, comment #5: 

Thanks, you are right. I will change the documentation.
I will check also what happened when udata,vdata are in descending order.

Avinoam Kalma <avinoam>
Group Member
Sun 29 Aug 2021 08:17:10 PM UTC, comment #4: 

output appears to now be correct vs matlab.  should the help text for xdata and ydata be updated to indicate the change from expected [min max]?  the matlab help indicates that it refers to [1st_row/col last_row/col] instead, implying the flip.

Nicholas Jankowski <nrjank>
Group Member
Sun 29 Aug 2021 06:57:20 PM UTC, comment #3: 

Please review the attached patch, which should fix this bug.
Attached: a new version of imtransform.m and a diff file

(file #51828, file #51829)

Avinoam Kalma <avinoam>
Group Member
Sun 31 Jan 2021 09:27:27 PM UTC, comment #2: 

Dear Anonymous, thank you for letting us know about this bug you found in Octave's image package.

Our image package is maintained by (very) few volunteers who donate their spare time to keep it up and running as well as improving it. I hope that one of us can have a closer look at this issue in the near future. But much to my regret, it often takes an unpleasantly long time before this happens.

If you are familiar with writing and debugging m-files, like our imtransform.m, then you could really help us to fix the bug you found by investigating yourself and trying to pin-point where and how our m-code should be improved. We would be really grateful for any contribution of yours to make the image package even better.

Hartmut <hardy>
Thu 28 Jan 2021 07:27:34 PM UTC, comment #1: 

adding people to cc

Hartmut <hardy>
Wed 27 Jan 2021 09:30:55 AM UTC, original submission:  


im = [1 2;3 4];
tmat = [eye(2); 0 0];
T = maketform ('affine', tmat);
im2 = imtransform (im, T, 'xdata', [3 1], 'ydata', [1 3]);


This code would result in [0 2 1; 0 4 3; 0 0 0] in Matlab but a 3x0 matrix in Octave.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51834:  imtransform_V2.diff added by avinoam (5KiB - application/octet-stream)
file #51835:  imtransform_V2.m added by avinoam (17KiB - application/octet-stream)
file #51828:  imtransform.m added by avinoam (16KiB - application/octet-stream)
file #51829:  imtransform.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 nrjank (Posted a comment)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by avinoam (Updated the item)
  • -email is unavailable- added by siko1056 (Updated the item)
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by hardy
  • -email is unavailable- added by hardy
  •  

    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 group members can vote.

     

    Follow 16 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-11-22 avinoam StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
    2021-11-11 avinoam StatusPatch Submitted Patch Reviewed
        Assigned toNone avinoam
    2021-08-30 avinoam Attached File- Added imtransform_V2.diff, #51834
        Attached File- Added imtransform_V2.m, #51835
        StatusConfirmed Patch Submitted
    2021-08-29 avinoam Attached File- Added imtransform.m, #51828
        Attached File- Added imtransform.diff, #51829
        Operating SystemGNU/Linux Any
        Carbon-Copy- Added pantxo
    2021-01-31 avinoam StatusNone Confirmed
        Release5.2.0 other
    2021-01-31 siko1056 Summaryimage package: imtransform xdata can't handle descending order [octave forge] (image) imtransform xdata can't handle descending order
    2021-01-28 hardy Carbon-Copy- Added avinoam
        Carbon-Copy- Added carandraug

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code