bugGNU Octave - Bugs: bug #46171, datevec fails with FFF and PM

 
 

bug #46171: datevec fails with FFF and PM

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Sat 10 Oct 2015 01:36:06 PM UTC
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Nick Jankowski Open/Closed:  * Closed
Release:  * 4.0.0 Operating System:  * Microsoft Windows
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Thu 22 Oct 2015 07:50:18 PM UTC, comment #10: 

no problem. Sure, stick my name on the list.

sorry for the extra work in formatting. I sort of knew it was lacking in that regard, just didn't get back to it to finish fixing. thanks for closing the loop.

Nicholas Jankowski <nrjank>
Project Member
Thu 22 Oct 2015 06:04:29 PM UTC, comment #9: 

Wow, thanks for both the discovery and solution to this problem.  I converted your patch to core Octave coding style.  For example, We use a space between a function name and the parenthesis that marks the start of the argument list.  The space is not used if you are doing indexing on a vector or matrix.  This makes it simple to tell the two situations apart.  The list of all our coding conventions is in one of the appendices of the Octave manual if you start generating more patches.  Do you wish to be listed as a contributor to OCtave in the front of the manual?  We like to acknowledge everyone who has helped, but some people prefer to remain anonymous.

The completed changeset was made on the stable branch (http://hg.savannah.gnu.org/hgweb/octave/rev/1a02e15a9f4c) and will be part of the 4.0.1 bug fix release.

Rik <rik5>
Project Administrator
Tue 13 Oct 2015 01:33:13 PM UTC, comment #8: 

verified that FFF behavior matches output with Matlab 2015a. (handles fewer than 3 digits whether at the end of the file or not)

believe the revised function should be good to go.

Nicholas Jankowski <nrjank>
Project Member
Mon 12 Oct 2015 02:26:02 AM UTC, comment #7: 

forgot to mod header

(file #35161)

Nicholas Jankowski <nrjank>
Project Member
Mon 12 Oct 2015 02:22:19 AM UTC, comment #6: 

a few more edits needed than I had anticipated to pass all the tests after my changes broke a couple. passes all tests now, and I added a few more to catch FFF, AM/PM, and variable number of FFF digits.

I haven't been able to compare the tests to expected Matlab output, someone should make sure there's nothing in the tests that shouldn't be there, or maybe add a couple extra.

Revised file appended to this comment.

(file #35160)

Nicholas Jankowski <nrjank>
Project Member
Sun 11 Oct 2015 12:56:01 PM UTC, comment #5: 

ok, replacing the lines 254-263 if block with:

  if (! isempty (idx))
    ## Kludge to handle FFF millisecond format since strptime does not
    msec = ds(idx+1:idx+3) #only works for 3digits in data, will fail for <3
    f(idx:idx+2) = msec#[];
    [~, nc] = strptime (ds, f);
    if (nc > 0)
      #msec = ds(nc:min(nc+2, end));
      #f = [f(1:idx-1) msec f(idx:end)];
      [tm, nc] = strptime (ds, f);
      tm.usec = 1000 * str2double (msec);
    endif

makes things work for having both FFF and am/pm. I left the commented out lines in the inner if block for reference. also, note that it will only work for 3 digits given for the FFF part. If less than three digits is given for FFF, it will grab extra trailing characters. previous code assumed the FFF data was either 3 digits or at the end of the string. not sure of the best way to cover that.  if there's an easy way to have it grab up to the first non-numeric or whitespace, then the line I threw into Comment 2 could take care of it for numel(msec)<3

Nicholas Jankowski <nrjank>
Project Member
Sun 11 Oct 2015 12:30:47 PM UTC, comment #4: 

correction. that line works, but the line 258 if block to get tm.usec breaks. maybe a kludge for the kludge...

Nicholas Jankowski <nrjank>
Project Member
Sun 11 Oct 2015 03:39:21 AM UTC, comment #3: 

appears that at line 257 strptime fails to match ds to format f, so returns nc=0. this makes it skips over the 'msec' kludge and the next loop as well, then return false for found, which calls the error.

it appears that when the .FFF is stripped out of the format string, the . is left.  if the fractional seconds are added as a string to format, as is done in the line 258 if block, then things work.

Nicholas Jankowski <nrjank>
Project Member
Sun 11 Oct 2015 03:02:43 AM UTC, comment #2: 

starting to step through datevec.m, stumbled onto the fact that inside the kludge it doesn't distinguish between .2 .02 and .002.  all three give the output of .002

i don't know if it should error out if FFF is given but less than three digits are there. I can see where some programs or systems might drop trailing zeros.

If we need to throw an error for ML compatibility, that should be easy enough by checking numel(msec)<3.

If it needs to function, inserting the following line before 262 fixes the problem:

msec = [msec,'0'(ones(1,3-numel(msec)))];

Nicholas Jankowski <nrjank>
Project Member
Sat 10 Oct 2015 01:38:08 PM UTC, comment #1: 

peeking at datevec.m, I see on line 255:

## Kludge to handle FFF millisecond format since strptime does not

haven't stepped through it all yet, but my guess is the kludge is missing a piece... :)  

Nicholas Jankowski <nrjank>
Project Member
Sat 10 Oct 2015 01:36:06 PM UTC, original submission:  

Was attempting to troubleshoot some code using datenum. it was failing on trying to read a particular datestring that had both fractional seconds and the PM designation. if I remove either, it works fine. Example:

>> datevec('06/01/2015 3:07:12 PM','mm/dd/yyyy HH:MM:SS PM')
ans =

   2015      6      1     15      7     12

>> datevec('06/01/2015 3:07:12.123','mm/dd/yyyy HH:MM:SS.FFF')
ans =

   2015.0000      6.0000      1.0000      3.0000      7.0000     12.1230

>> datevec('06/01/2015 3:07:12.123 PM','mm/dd/yyyy HH:MM:SS.FFF PM')
error: datevec: DATE not parsed correctly with given format
error: called from
    datevec at line 147 column 11

Note: don't have ML access right now to verify expected/compatible ML behavior.

(a side point: should datevec fail (for compatibility) if the month and day are not zero padded? From the help I thought it should, but it does not?)

>> datevec('06/01/2015 3:07:12 PM','mm/dd/yyyy HH:MM:SS PM')
ans =

   2015      6      1     15      7     12

>> datevec('6/1/2015 3:07:12 PM','mm/dd/yyyy HH:MM:SS PM')
ans =

   2015      6      1     15      7     12

Nicholas Jankowski <nrjank>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #35161:  datevec.m added by nrjank (13KiB - application/octet-stream - corrected datevec.m with edited header info)
file #35160:  datevec.m added by nrjank (13KiB - application/octet-stream - edited datevec.m, bug should be corrected, all tests passed)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

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

    Date Changed by Updated Field Previous Value => Replaced by
    2015-10-22 rik5 StatusNone Fixed
        Open/ClosedOpen Closed
    2015-10-12 nrjank Attached File- Added datevec.m, #35161
    2015-10-12 nrjank Attached File- Added datevec.m, #35160

    Back to the top


    Powered by Savane 3.9