times.parse gives the wrong day of the week for the first hour of the day. #4922

Closed
jdbernard opened this Issue Oct 21, 2016 · 3 comments

Projects

None yet

1 participant

@jdbernard
Contributor

The following:

import times
echo $parse("2015-12-31 00:00:00", "yyyy-MM-dd HH:mm:ss")

Yields:

Wed Dec 31 00:00:00 2015

Every calendar I've consulted has shown Dec. 31st, 2015 to be a Thursday, not a Wednesday. Additionally, changing from 2015-12-31 00:00:00 to 2015-12-31 01:00:00 corrects the output, yielding Thu Dec 31 01:00:00 2015.

I know there is ambiguity with regard to on which date midnight itself falls (NIST says that exact midnight is ambiguous and could belong either day), but that is not at issue here since we are specifying exactly the date as the 31st. Further, it might make sense for Nim to possibly interpret it as midnight on Wed Dec 30 or midnight Thu Dec 31, but Wed Dec 31 is obviously incorrect. The fact that 2015-12-31 00:01:00, which unambiguously falls on Thursday Dec. 31st is interpreted as Wed Dec 31 as well further points to the incorrect behavior.

This also does not appear to be specific to Dec. 31st, 2015. Every date of the several I have tried exhibits this behavior.

I've replicated this behavior on Linux (Ubuntu 16.04), OSX 10.12, and OpenBSD 5.6, so it seems consistent across a couple of systems.

@jdbernard
Contributor

After further testing it looks like it may be related to DST. Specifically, parse is not setting DST appropriately. It seems to be using the DST value of the current timezone relative to the current date and not relative to the date being parsed.

Example (to be run during DST):

import times

let t1 = parse("2016-01-01 12:00:00", "yyyy-MM-dd HH:mm:ss")

# will fail
# assert (not t1.isDST)

echo $t1 & (if t1.isDST: " (DST)" else: "")
@jdbernard
Contributor
jdbernard commented Oct 21, 2016 edited

This procedure works around the bug in times.parse:

proc fixedParse*(value, format: string): TimeInfo =
  let firstParsed = parse(value, format)
  let rectified = getLocalTime(toTime(firstParsed))
  if firstParsed.isDST and not rectified.isDST: return rectified + 1.hours
  elif not firstParsed.isDST and rectified.isDST: return rectified - 1.hours
  else: return rectified

Transforming it to a Time and back to TimeInfo correctly sets the DST setting, then we correct for the time offset that was mistakenly applied if we had to correct the DST setting.

I'll work on a patch to fix times.parse directly.

@jdbernard
Contributor

PR #4923 is open for a fix in times.

@dom96 dom96 closed this in b7232bd Oct 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment