fread: Improve validation of the dec argument#7738
fread: Improve validation of the dec argument#7738mcol wants to merge 2 commits intoRdatatable:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7738 +/- ##
=======================================
Coverage 99.04% 99.04%
=======================================
Files 87 87
Lines 17058 17058
=======================================
Hits 16895 16895
Misses 163 163 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| test(2370.5, yearmon(NA, format="character"), NA_character_) | ||
|
|
||
| ## validation of dec argument | ||
| test(7737.1, fread(input = "whatever.csv", dec = NA_character_), error="nchar(dec) == 1L is not TRUE") |
There was a problem hiding this comment.
this error comes from base, not data.table, so the test is fragile. Look around and especially near the top of the test file for our proposed way to structure such tests.
| } | ||
| stopifnot( is.character(dec), length(dec)==1L) | ||
| if (dec == "auto") dec = "" else stopifnot(nchar(dec) == 1L) | ||
| if (!is.na(dec) && dec == "auto") dec = "" else stopifnot(nchar(dec) == 1L) |
There was a problem hiding this comment.
this also gives the same "ugly" error for dec = letters, so might as well change to if (isTRUE(dec == "auto")) or if (identical(dec, "auto")).
There was a problem hiding this comment.
The case dec = letters is already handled by the stopifnot() just before it. But using one of your variations is nicer anyway.
2 participants
Fixes #7737 and adds a test.