Skip to content

Commit

Permalink
Use hms class rather than custom time class
Browse files Browse the repository at this point in the history
  • Loading branch information
jimhester committed Jun 8, 2016
1 parent 6cea074 commit ae77f30
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 34 deletions.
3 changes: 2 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ LinkingTo: Rcpp,
Imports:
Rcpp (>= 0.11.5),
curl,
tibble
tibble,
hms
Suggests:
testthat,
knitr,
Expand Down
2 changes: 0 additions & 2 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ S3method(as.col_spec,character)
S3method(as.col_spec,col_spec)
S3method(as.col_spec,default)
S3method(as.col_spec,list)
S3method(format,time)
S3method(output_column,POSIXt)
S3method(output_column,default)
S3method(output_column,double)
S3method(print,col_spec)
S3method(print,collector)
S3method(print,date_names)
S3method(print,locale)
S3method(print,time)
export(col_character)
export(col_date)
export(col_datetime)
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# readr 0.2.2.9000
* time objects returned by `parse_time()` are now `hms` objects rather than a
custom `time` class (#409, @jimhester).

* The flexible time parser now requires colons between hours and minutes and
optional seconds. (#424, @jimhester)
Expand Down
20 changes: 0 additions & 20 deletions R/time.R

This file was deleted.

8 changes: 4 additions & 4 deletions src/Collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,22 +341,22 @@ void CollectorTime::setValue(int i, const Token& t) {

if (!res) {
warn(t.row(), t.col(), "time like " + format_, std_string);
INTEGER(column_)[i] = NA_INTEGER;
REAL(column_)[i] = NA_REAL;
return;
}

DateTime dt = parser_.makeTime();
if (!dt.validTime()) {
warn(t.row(), t.col(), "valid date", std_string);
INTEGER(column_)[i] = NA_INTEGER;
REAL(column_)[i] = NA_REAL;
return;
}
INTEGER(column_)[i] = dt.time();
REAL(column_)[i] = dt.time();
return;
}
case TOKEN_MISSING:
case TOKEN_EMPTY:
INTEGER(column_)[i] = NA_INTEGER;
REAL(column_)[i] = NA_REAL;
return;
case TOKEN_EOF:
Rcpp::stop("Invalid token");
Expand Down
5 changes: 3 additions & 2 deletions src/Collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class CollectorTime : public Collector {

public:
CollectorTime(LocaleInfo* pLocale, const std::string& format):
Collector(Rcpp::IntegerVector()),
Collector(Rcpp::NumericVector()),
format_(format),
parser_(pLocale)
{
Expand All @@ -217,7 +217,8 @@ class CollectorTime : public Collector {
void setValue(int i, const Token& t);

Rcpp::RObject vector() {
column_.attr("class") = "time";
column_.attr("class") = Rcpp::CharacterVector::create("hms", "difftime");
column_.attr("units") = "secs";
return column_;
};

Expand Down
10 changes: 5 additions & 5 deletions tests/testthat/test-parsing-time.R
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
context("Parsing, time")

test_that("default format captures cases", {
late_night <- structure(22 * 3600 + 20 * 60, class = "time")
late_night <- hms::hms(seconds = 22 * 3600 + 20 * 60)
expect_equal(parse_time("22:20"), late_night)
expect_equal(parse_time("10:20 pm"), late_night)

expect_equal(parse_time("22:20:05"), late_night + 5)
expect_equal(parse_time("10:20:05 pm"), late_night + 5)
expect_equal(parse_time("22:20:05"), hms::as.hms(late_night + 5))
expect_equal(parse_time("10:20:05 pm"), hms::as.hms(late_night + 5))
})

test_that("parses NA/empty correctly", {
out <- parse_time(c("NA", ""))
exp <- structure(c(NA_real_, NA_real_), class = "time")
exp <- hms::hms(seconds = c(NA_real_, NA_real_))
expect_equal(out, exp)

expect_equal(parse_time("TeSt", na = "TeSt"),
structure(NA_real_, class = "time"))
hms::hms(seconds = NA_real_))
})

test_that("times are guessed as expected", {
Expand Down

0 comments on commit ae77f30

Please sign in to comment.