bugGNU Octave - Bugs: bug #53378, Octave date function doesn't...

 
 

bug #53378: Octave date function doesn't provide any error when provided arguments where MATLAB does

Submitter:  Dildar Sk <hodor123456>
Submitted:  Mon 19 Mar 2018 04:15:22 AM UTC
   
 
Category:  Octave Function Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Missed Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Dildar Sk Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 22 Mar 2018 07:18:01 AM UTC, comment #6: 

I adapted your latest change and pushed it on the stable branch. Thank you for your contribution to Octave!

Functionally your patch was absolutely correct. But stylistically, I think you should study the Octave code base and our codind style guidelines a little more. Please review the changes that I made from the patch you provided. You were still missing a blank line after the input validation if-endif block. We typically label tests that test input validation "## Test input validation". The %!error call does not need an extra set of parentheses. And I reworded the commit message to resemble other commit messages of this type as I tried to point out to you.

Mike Miller <mtmiller>
Group Member
Wed 21 Mar 2018 09:04:40 AM UTC, comment #5: 

Updated the function as like beep.m
Updated the commit message and added a test as you told.
Please review if anything still missing there.
Patch added.

(file #43612)

Dildar Sk <hodor123456>
Wed 21 Mar 2018 08:27:21 AM UTC, comment #4: 

Please compare your change to the Octave function beep.m. The argument checking should look exactly the same, both functions take no arguments.

You should also write your commit message according to https://wiki.octave.org/Commit_message_guidelines.

An example of a similar commit to this one is https://hg.savannah.gnu.org/hgweb/octave/rev/4591a1238ee0.

You might also want to add a test case to ensure that this error check works, for example


## Test input validation
%!error date (1)


Mike Miller <mtmiller>
Group Member
Mon 19 Mar 2018 07:41:10 AM UTC, comment #3: 

Here I updated the patch and tried to follow the conventions.
Please review if there is any error.Thanks.

(file #43593)

Dildar Sk <hodor123456>
Mon 19 Mar 2018 06:47:05 AM UTC, comment #2: 

Thanks for the report and the patch. This is a valid change to make, and your patch almost does this correctly. All Octave functions use the print_usage function when reporting an error for an incorrect number of arguments, please change that.

Could you please review the Octave coding style guide (https://wiki.octave.org/Octave_style_guide) and other m-file functions in Octave and try to conform to the existing conventions?

Mike Miller <mtmiller>
Group Member
Mon 19 Mar 2018 04:18:33 AM UTC, comment #1: 

The m-file has some problems.So I changed it.Patch attached.

(file #43591)

Dildar Sk <hodor123456>
Mon 19 Mar 2018 04:15:22 AM UTC, original submission:  

In Octave when you type

>> date()

ans = 19-Mar-2018

>> date('today')

ans = 19-Mar-2018

>> date('tomorrow')

ans = 19-Mar-2018

>> date('whatever')

ans = 19-Mar-2018

>>


It neither provide right answer nor any error,
Where in MATLAb,

>> date('today')

Error using date
Too many input arguments.
 

>> date('today')

Error using date
Too many input arguments.
 

>> date('tomorrow')

Error using date
Too many input arguments.
 

>> date


ans =

    '19-Mar-2018'

>>
>>

Dildar Sk <hodor123456>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43593:  date_error_new.patch added by hodor123456 (582B - text/x-patch)
file #43591:  date_error.patch added by hodor123456 (599B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-03-22 mtmiller StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
    2018-03-21 hodor123456 Attached File- Added date_error_latest.patch, #43612
    2018-03-21 mtmiller StatusNone Patch Reviewed
    2018-03-19 hodor123456 Attached File- Added date_error_new.patch, #43593
    2018-03-19 mtmiller Severity3 - Normal 2 - Minor
        Item GroupMatlab Compatibility Missed Error or Warning
    2018-03-19 hodor123456 Attached File- Added date_error.patch, #43591
    2018-03-19 hodor123456 Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code