Added format specifier for yearqtr#7713
Added format specifier for yearqtr#7713LunaticSage218 wants to merge 7 commits intoRdatatable:masterfrom
Conversation
LunaticSage218
requested review from
MichaelChirico and
ben-schwen
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7713 +/- ##
==========================================
- Coverage 99.04% 99.04% -0.01%
==========================================
Files 87 87
Lines 17037 17138 +101
==========================================
+ Hits 16874 16974 +100
- Misses 163 164 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Benjamin Schwendinger <52290390+ben-schwen@users.noreply.github.com>
|
Solid first approach. Keep in mind that we strive for efficiency with |
| test(2368.3, yearqtr(x, format="character"), c("1111Q4", "2019Q1", "2019Q1", "2019Q1", "2019Q4", "2020Q1", "2020Q1", "2020Q4", "2040Q1", "2040Q4", "2100Q1", NA_character_)) | ||
| test(2368.4, yearqtr("2016-08-03 01:02:03.45", format="character"), "2016Q3") | ||
| test(2368.5, yearqtr(NA, format="character"), NA_character_) | ||
| test(2368.6, yearqtr(x, format="invalid"), error="should be one of") |
There was a problem hiding this comment.
Pls delete test case 6, since you are essentially testing for the R error message of match.arg, which is beyond our control
| yearmon(x) | ||
| yearqtr(x) | ||
|
|
||
| yearqtr(x, format = c("numeric", "character")) |
There was a problem hiding this comment.
| yearqtr(x, format = c("numeric", "character")) | |
| yearqtr(x, format = c("numeric", "character")) | |
reinstate deletion of missing line
ben-schwen
left a comment
ben-schwen
left a comment
There was a problem hiding this comment.
We are close to being mergeable.
However, to fully close the issue you would also need to mirror the approach for yearmon with a similar encoding d="2019-01-01" as yearmon(d, "character") as "2019M1"
| if (format == "numeric") return(yqtr) | ||
| yr = floor(yqtr) | ||
| qtr = round((yqtr - yr) * 4) + 1L | ||
| ans = paste0(yr, "Q", qtr) |
There was a problem hiding this comment.
I would go with allocating NA vector first, and then filling up non-NA, less input to paste, which needs to populate global string cache on each string.
3 participants
Towards #7694