bugGNU Octave - Bugs: bug #52380, lower/upper functions should pass...

 
 

bug #52380: lower/upper functions should pass any numeric value?

Submitted by:  A.R. Burgers <arb>
Submitted on:  Thu 09 Nov 2017 10:08:51 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  Closed
Release:  dev Operating System:  Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 11 Jan 2018 08:52:57 PM UTC, comment #6:

All done. I checked in your changeset here (http://hg.savannah.gnu.org/hgweb/octave/rev/6ece7c7e4912). Thanks for the code!

Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Wed 10 Jan 2018 11:04:30 PM UTC, comment #5:

Thanks Rik. I'll make sure to make tests in the future. In the mean time, I've updated the patch you made to include the update to the missing file.

(file #42885)

Stevie Schraudner <stevietodd>
Fri 05 Jan 2018 06:29:57 AM UTC, comment #4:

@Stevie: I modified your changeset slightly. Octave follows the GNU coding guidelines for commit messages which are a one-line summary (which you had) followed by a per file/per function explanation of what was changed. I added the second bit. We also try, when we're being good, to use test-driven development. So I wrote BIST tests for toupper and tolower and included them in mappers.cc (where the two functions reside). I then compiled and ran

which proves why tests are important. Apparently the float scalar case in ov-flt-complex.cc is missing.

I uploaded the most recent version of your cset so you can patch it to work for all possible inputs.

(file #42841)

Rik <rik5>
Project Administrator
Wed 27 Dec 2017 02:52:35 PM UTC, comment #3:

I'm new to contributing so I'm not sure if I'm following the process correctly, but I have attached a changeset at the bottom that should fix this bug. Thanks.

(file #42738)

Stevie Schraudner <stevietodd>
Mon 13 Nov 2017 07:17:37 PM UTC, comment #2:

from an examination of the code, it appears making lower/upper work on complex numbers entails implementing the umap_xtolower/umap_xtoupper to files ov-cx-mat.cc, ov-flt-cx-mat.cc and complex.cc in libinterp/octave-value.

A.R. Burgers <arb>
Sat 11 Nov 2017 01:08:02 AM UTC, comment #1:

I can confirm this for R2017b, even though the Matlab documentation does not mention this use case:

https://www.mathworks.com/help/matlab/ref/lower.html

For what I see, this item is independent of bug #52358, but it is an example where this behavior was triggered.

Kai Torben Ohlhus <siko1056>
Project Member
Thu 09 Nov 2017 10:08:51 PM UTC, original submission:

See http://savannah.gnu.org/bugs/?52358

There it appears tolower fails on complex numeric values.
I guess lower/upper should return any numeric value unchanged:

On matlab

octave:

A.R. Burgers <arb>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #42885:  52380.cset added by stevietodd (4KiB - application/octet-stream - Thanks RIk. I'll make sure to add tests next time. For now, I've updated your patch to include the update to the missing file.)
file #42841:  52380.cset added by rik5 (4KiB - application/octet-stream)
file #42738:  mypatch.patch added by stevietodd (2KiB - text/x-patch - Changeset to fix this bug)
file #42737:  mypatch.patch added by stevietodd (2KiB - text/x-patch - Changeset to fix this bug)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by stevietodd (Updated the item)
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by arb (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-01-11 rik5 StatusPatch Reviewed => Fixed
        Open/ClosedOpen => Closed
    2018-01-10 stevietodd Attached File- => Added 52380.cset, #42885
    2018-01-05 rik5 Attached File- => Added 52380.cset, #42841
        StatusConfirmed => Patch Reviewed
    2017-12-27 stevietodd Attached File- => Added mypatch.patch, #42738
    2017-12-27 stevietodd Attached File- => Added mypatch.patch, #42737
    2017-11-11 siko1056 StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.3