parse_time() now correctly handles 12 AM/PM (#584)

Fixes #579
tidyverse · Jan 24, 2017 · 7ef248a · 7ef248a
1 parent 3fd00e3
commit 7ef248a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 2 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 # readr 1.0.0.9000
 
+* `parse_time()` now correctly handles 12 AM/PM (#579).
+
 * `read_csv2()` gives a message if it updates the default locale (#443, @krlmlr).
 
 * `col_euro_double()`, `parse_euro_double()`, `col_numeric()`, and 

diff --git a/src/DateTimeParser.h b/src/DateTimeParser.h
@@ -311,7 +311,24 @@ class DateTimeParser {
 
 private:
   int hour() {
-    return hour_ + (amPm_ == 1 ? 12 : 0);
+    if (hour_ == 12) {
+
+      // 12 AM
+      if (amPm_ == 0) {
+        return hour_ - 12;
+      }
+
+      // 12 PM
+      return hour_;
+    }
+
+    // Rest of PM
+    if (amPm_ == 1) {
+      return hour_ + 12;
+    }
+
+    // 24 hour time
+    return hour_;
   }
 
   inline bool consumeSeconds(int* pSec, double* pPartialSec) {
@@ -481,7 +498,7 @@ class DateTimeParser {
     min_ = 0;
     sec_ = 0;
     psec_ = 0;
-    amPm_ = 0;
+    amPm_ = -1;
     compactDate_ = true;
 
     tzOffsetHours_ = 0;

diff --git a/tests/testthat/test-parsing-time.R b/tests/testthat/test-parsing-time.R
@@ -9,6 +9,15 @@ test_that("default format captures cases", {
   expect_equal(parse_time("10:20:05 pm"), hms::as.hms(late_night + 5))
 })
 
+test_that("twelve o'clock is parsed properly", {
+  morning <- hms::hms(seconds = 0 * 3600 + 1 * 60)
+  midday <- hms::hms(seconds = 12 * 3600 + 1 * 60)
+
+  expect_equal(parse_time("12:01 AM"), morning)
+  expect_equal(parse_time("12:01 PM"), midday)
+  expect_equal(parse_time("12:01"), midday)
+})
+
 test_that("accepts single digit hour", {
   early_morn <- hms::hms(seconds = 1 * 3600 + 20 * 60)
   expect_equal(parse_time("1:20 am"), early_morn)