Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hms class rather than custom time class #425

Merged
merged 1 commit into from
Jun 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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