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.

Sign up for GitHub

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

add %Y M %M and %M M %Y formats #221

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

add %Y M %M and %M M %Y formats #221

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
64dfef6
add %Y M %M and %M M %Y formats
davidtedfordholt Sep 26, 2020
ce72c12
add tests for %Ym%M yearmonth()
davidtedfordholt Sep 28, 2020
98e5128
Merge pull request #1 from davidtedfordholt/patch-2
davidtedfordholt Sep 28, 2020
b5562ac
save regular expression to variable
davidtedfordholt Sep 28, 2020
16ba7be
remove [[:space:]]* from regex
davidtedfordholt Sep 29, 2020
d23867e
add ctb
davidtedfordholt Sep 29, 2020
f8806fe
add news for %Y M%M
davidtedfordholt Sep 29, 2020
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
23 changes: 21 additions & 2 deletions R/yearmonth.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,27 @@ yearmonth.Date <- function(x) {

#' @export
yearmonth.character <- function(x) {
assertDate(x)
new_yearmonth(anydate(x))
key_words <- regmatches(x, gregexpr("[[:alpha:]]+", x))
search_expr <- "^[[:digit:]]{1~4}[[:space:]]*(m|mon|month)[[:space:]]*[[:digit:]]{1~4}$"
if (all(grepl(search_expr, key_words, ignore.case = TRUE))) {
yr_mon <- regmatches(x, gregexpr("[[:digit:]]+", x))
digits_lgl <- map_lgl(yr_mon, ~ !has_length(.x, 2))
digits_len <- map_int(yr_mon, ~ sum(nchar(.x)))
digits_ind <- nchar(flatten_chr(yr_mon))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line exactly do? Why it can't be length of 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we allow years before 1000AD using this format, there is no way to tell the difference between %y and %Y for 2-digit years. Also, it seems more likely that a 3-digit year specification is an error than that it refers to a year before 1000AD (especially using this format).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay to throw an error for 3-digit years for now. If there's a compelling use case using 3-digit years, we'll accomodate this later.

if (any(digits_lgl) || any(digits_len < 5) || any(digits_len > 6) || 3 == digits_ind) {
abort("Character strings are not in a standard unambiguous format.")
}
yr_lgl <- map(yr_mon, ~ grepl("[[:digit:]]{4}", .x))
yr <- as.integer(map2_chr(yr_mon, yr_lgl, ~ .x[.y]))
mon <- as.integer(map2_chr(yr_mon, yr_lgl, ~ .x[!.y]))
if (any(mon > 12)) {
abort("Months can't be greater than 12.")
}
yearmonth(12 * (yr - 1970) + mon - 1)
} else {
assertDate(x)
new_yearmonth(anydate(x))
}
Comment on lines +81 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (any(mon > 12)) {
abort("Months can't be greater than 12.")
}
yearmonth(12 * (yr - 1970) + mon - 1)
} else {
assertDate(x)
new_yearmonth(anydate(x))
}
x <- paste(yr, mon)
}
assertDate(x)
new_yearmonth(anydate(x))

}

#' @export
Expand Down
11 changes: 11 additions & 0 deletions tests/testthat/test-yearmonth.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ test_that("input types for yearmonth()", {
expect_identical(yearmonth(c(596, 576)), expected)
})

test_that("%Ym%M format for yearmonth()", {
expect_identical(yearmonth("2018 M01"), yearmonth("2018 Jan"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unit test fails on gh CI.

expect_identical(yearmonth("2018 M1"), yearmonth("2018 Jan"))
expect_identical(yearmonth("2018mon01"), yearmonth("2018 Jan"))
expect_identical(yearmonth("2018 month 12"), yearmonth("2018 Dec"))
expect_error(yearmonth("201M1"))
expect_error(yearmonth("2018M13"))
expect_error(yearmonth("201M811"))
Comment on lines +20 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include the error msg you expect?

expect_error(yearmonth(c("2018M1", "2018 Feb", "2018M3")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think they are valid inputs, although a mixture of styles. I'll leave comments above to allow for this.

})

test_that("vec_arith() for yearmonth()", {
expect_identical(yearmonth(x) + 1:2, yearmonth(c("2019 Oct", "2018 Mar")))
expect_identical(yearmonth(x) - 1, yearmonth(c("2019 Aug", "2017 Dec")))
Expand Down