Skip to content

Commit

Permalink
Display the full field in the problems result
Browse files Browse the repository at this point in the history
Some parsers, notably the numeric parser modify the SourceIterator pointers when
parsing, so using the same object in the problem display only shows part
of the original field. We now create a new object when adding warnings
to ensure the full field is shown.

Fixes #444 (comment)
  • Loading branch information
jimhester committed Oct 24, 2019
1 parent b73dbbc commit 0fd92f8
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# readr (development version)

* The full problem field is now displayed in the problems tibble, as intended
(#444).

* `guess_parser()` gains a `na` argument and removes NA values before guessing.
`parse_guess()` now passes the `na` argument to `guess_parser()` (#1041).

Expand Down
15 changes: 10 additions & 5 deletions src/Collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,15 @@ void CollectorDouble::setValue(int i, const Token& t) {
parseDouble(decimalMark_, str.first, str.second, REAL(column_)[i]);
if (!ok) {
REAL(column_)[i] = NA_REAL;
warn(t.row(), t.col(), "a double", str);
SourceIterators org_str = t.getString(&buffer);
warn(t.row(), t.col(), "a double", org_str);
return;
}

if (str.first != str.second) {
REAL(column_)[i] = NA_REAL;
warn(t.row(), t.col(), "no trailing characters", str);
SourceIterators org_str = t.getString(&buffer);
warn(t.row(), t.col(), "no trailing characters", org_str);
return;
}

Expand Down Expand Up @@ -251,12 +253,14 @@ void CollectorInteger::setValue(int i, const Token& t) {
bool ok = parseInt(str.first, str.second, INTEGER(column_)[i]);
if (!ok) {
INTEGER(column_)[i] = NA_INTEGER;
warn(t.row(), t.col(), "an integer", str);
SourceIterators org_str = t.getString(&buffer);
warn(t.row(), t.col(), "an integer", org_str);
return;
}

if (str.first != str.second) {
warn(t.row(), t.col(), "no trailing characters", str);
SourceIterators org_str = t.getString(&buffer);
warn(t.row(), t.col(), "no trailing characters", org_str);
INTEGER(column_)[i] = NA_INTEGER;
return;
}
Expand Down Expand Up @@ -316,8 +320,9 @@ void CollectorNumeric::setValue(int i, const Token& t) {
decimalMark_, groupingMark_, str.first, str.second, REAL(column_)[i]);

if (!ok) {
SourceIterators org_str = t.getString(&buffer);
REAL(column_)[i] = NA_REAL;
warn(t.row(), t.col(), "a number", str);
warn(t.row(), t.col(), "a number", org_str);
return;
}

Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/test-problems.R
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,10 @@ test_that("problems returns the filename (#581)", {
expect_equal(length(files), 28L)
expect_equal("mtcars.csv'", basename(files)[[1L]])
})

test_that("problems returns full original field (#444)", {
probs <- problems(read_tsv("X\n-$12,500\n$2,000\n-$5,000\n$1,000\n-$3,000\n", col_types = list(.default = col_number())))

expect_equal(NROW(probs), 3)
expect_equal(probs$actual, c("-$12,500", "-$5,000", "-$3,000"))
})

0 comments on commit 0fd92f8

Please sign in to comment.