bugGNU Octave - Bugs: bug #47627, datenum input validation should be...

 
 

bug #47627: datenum input validation should be stricter

Submitted by:  None
Submitted on:  Tue 05 Apr 2016 04:36:40 PM UTC  
 
Category: Octave FunctionSeverity: 2 - Minor
Priority: 3 - LowItem Group: Documentation
Status: In ProgressAssigned to: None
Originator Name: Ruben GonzalezOriginator Email: -unavailable-
Open/Closed: OpenRelease: 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 Jump to the original submission)

Sat 28 Jan 2017 10:27:02 AM UTC, comment #14:

The latest patch by Lachlan solved the error. Thank you for your help.
I apologize for the late reply.

-Deleted Account- <carpediem>
Sat 02 Jul 2016 01:43:23 AM UTC, comment #13:

Aditya, I think Mike was wanting you to check for multiple occurrences of HH etc.

The attached patch is more thorough, checking that we don't have both YY and YYYY, for example.

I'm retagging from "Patch submitted" to "In progress", since this isn't a blocker for 4.2.0.

(file #37658)

Lachlan Andrew <lachlan>
Project Member
Sat 30 Apr 2016 04:25:59 PM UTC, comment #12:

IIRC, the standard date formats to be parsed (as defined inside of datevec) should show an error in case of conditions such as more than one occurrence of 'HH'. Please correct me if I am missing any possible test case. Updated diff file is attached.

(file #37030)

-Deleted Account- <carpediem>
Fri 29 Apr 2016 07:43:06 PM UTC, comment #11:

The attached patch checks whether "MM" appears more than once in the format string. I don't know enough to know whether this is the only change needed or if there are other similar cases that should be checked for. What about more than one occurrence of "HH"?

The patch contains some style problems, please make sure to use proper indentation, whitespace, and commenting style.

Also please provide a correct commit message, see for examples https://www.gnu.org/software/octave/doc/v4.0.1/General-Guidelines.html

Mike Miller <mtmiller>
Project Administrator
Sun 10 Apr 2016 08:15:05 AM UTC, comment #10:

Added a check for input if more than one MM identifiers exist. Function reports an error, as required. This patch is based on current tip, and is independent of the previous patch.

(file #36886)

-Deleted Account- <carpediem>
Sun 10 Apr 2016 06:23:09 AM UTC, comment #9:

So basically there should be a check to see if the string has mm and MM, both present. If the condition is true, octave should return an error notifying the user about the correct format. Did I get it correctly?

-Deleted Account- <carpediem>
Sat 09 Apr 2016 03:22:46 PM UTC, comment #8:

This isn't the right solution. It is an error to have more than one instance of the 'MM' identifier. See comment #2 where this was verified on Matlab. Octave needs to produce an error when this condition is detected rather than continue on using an unknown or assumed value for the month.

Rik <rik5>
Project Administrator
Sat 09 Apr 2016 02:50:46 PM UTC, comment #7:

Please add BISTs (built-in self-test) for this cases. See the bottom of datevec and datenum files for examples.

Carnë Draug <carandraug>
Project Member
Sat 09 Apr 2016 08:28:03 AM UTC, comment #6:

The patch is here
https://savannah.gnu.org/bugs/download.php?file_id=36871

-Deleted Account- <carpediem>
Sat 09 Apr 2016 04:40:51 AM UTC, comment #5:

The problem was not in datenum(), but in datevec().
Upon testing, I found that that datevec failed to determine the month value at all when MM is used instead of mm, as was evident from the following testcases:

datevec('2015-12-27 0:00','yyyy-MM-dd HH:MM')
datevec('2015-11-27 0:00','yyyy-mm-dd HH:MM')
datevec('2015-11-27 0:00','yyyy-MM-dd HH:MM')

The problem was that datevec was confusing 'MM' for minutes when passed as uppercase in the string and was not determining the value of month passed in f[]. It simply defaulted to month=1 for all cases, hence, giving wrong output to datenum(), which in turn calculated wrong output itself.

After applying this patch, the function works correctly according to all the std formats to be parsed, as defined in the file.

-Deleted Account- <carpediem>
Tue 05 Apr 2016 09:44:56 PM UTC, comment #4:

So it seems that Octave processes correct input accurately, but could do more to detect when the input is invalid. I'm going to change the bug summary to reflect that and mark the bug as confirmed.

Rik <rik5>
Project Administrator
Tue 05 Apr 2016 07:59:45 PM UTC, comment #3:

fixing the date portion to lower case, Matlab 2015b gives the following:

Nicholas Jankowski <nrjank>
Tue 05 Apr 2016 07:57:26 PM UTC, comment #2:

just checked Matlab 2015b. errors on all three:

Nicholas Jankowski <nrjank>
Tue 05 Apr 2016 05:10:15 PM UTC, comment #1:

Do you have access to Matlab and can you test there as well?

Second, according to the documentation for datestr, your format is invalid. The correct designation for years, months, and days is lower case.

It does look as if the input validation could be improved in Octave.

Rik <rik5>
Project Administrator
Tue 05 Apr 2016 04:36:40 PM UTC, original submission:

the function datenum for some reason ignores the month

For example, performing a datenum for March 31st 2015 yields

datenum('2015-03-31 0:00','YYYY-MM-DD HH:MM')
ans = 735995

Now for doing it for the next day, April 1st, we get

datenum('2015-04-01 0:00','YYYY-MM-DD HH:MM')
ans = 735965

which is 30 days LOWER than the previous result. Next, use June 1st instead

datenum('2015-05-01 0:00','YYYY-MM-DD HH:MM')
ans = 735965

You get the VERY SAME RESULT, which means that the month is not factored into datenum at all.

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #37030:  47627.diff added by carpediem (1KiB - text/x-patch)
file #36886:  47627.diff added by carpediem (1006B - text/x-patch)
file #36871:  patch47627.diff added by carpediem (781B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by lachlan (Updated the item)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by carpediem (Posted a comment)
  • -unavailable- added by nrjank (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by None (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 14 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 02 Jul 2016 01:43:23 AM UTClachlanAttached File-=>Added bug_47627_datevec_validation.cset, #37658
      StatusPatch Submitted=>In Progress
    Sat 30 Apr 2016 04:25:59 PM UTCcarpediemAttached File-=>Added 47627.diff, #37030
    Fri 29 Apr 2016 07:43:06 PM UTCmtmillerSeverity3 - Normal=>2 - Minor
      Priority5 - Normal=>3 - Low
      Item GroupIncorrect Result=>Documentation
      StatusConfirmed=>Patch Submitted
      Release4.0.1=>dev
      Operating SystemMicrosoft Windows=>Any
    Sun 10 Apr 2016 08:15:05 AM UTCcarpediemAttached File-=>Added 47627.diff, #36886
    Sat 09 Apr 2016 04:41:22 AM UTCcarpediemAttached File-=>Added patch47627.diff, #36871
    Tue 05 Apr 2016 09:44:56 PM UTCrik5StatusNeed Info=>Confirmed
      Summarydatenum gives wrong result, ignores month=>datenum input validation should be stricter
    Tue 05 Apr 2016 05:10:15 PM UTCrik5StatusNone=>Need Info

    Back to the top


    Powered by Savane 3.1-cleanup1