Skip to content

Parse int fields as double so large values don't get set to NA #237

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

Merged
nmdefries merged 5 commits into from
Jan 18, 2024
Merged

Parse int fields as double so large values don't get set to NA #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
543951c
parse int as double so large values don't get set to NA
nmdefries Jan 10, 2024
0cdd4e9
docs: document (GHA)
nmdefries Jan 10, 2024
bac1ea2
warn if rounding changes int value; tests
nmdefries Jan 10, 2024
312f8f1
style: styler (GHA)
nmdefries Jan 10, 2024
a25797f
linting
nmdefries Jan 10, 2024
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
14 changes: 13 additions & 1 deletion R/model.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,19 @@ parse_value <- function(info, value, disable_date_parsing = FALSE) {
} else if (info$type == "bool") {
return(as.logical(value))
} else if (info$type == "int") {
return(as.integer(value))
# Int doesn't have enough capacity to store some weekly `pub_wiki` values.
value <- as.double(value)
if (any(value != round(value))) {
cli::cli_warn(
c(
"Values in {info$name} were expected to be integers but contain a decimal component",
"i" = "Decimal components are returned as-is"
),
class = "epidatr__int_nonzero_decimal_digits"
)
}

Copy link
Contributor

@dshemetov dshemetov Jan 11, 2024

Choose a reason for hiding this comment

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

Could use this: https://mllg.github.io/checkmate/reference/checkIntegerish.html

Copy link
Contributor Author

@nmdefries nmdefries Jan 18, 2024

Choose a reason for hiding this comment

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

Thanks, that's cleaner. Unfortunately, their check is too thorough -- it also fails when values are too large to be stored as ints, which is the case here.

Copy link
Contributor

@dshemetov dshemetov Jan 18, 2024

Choose a reason for hiding this comment

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

Oof, too bad

return(value)
} else if (info$type == "float") {
return(as.double(value))
} else if (info$type == "categorical") {
Expand Down
4 changes: 2 additions & 2 deletions man/covidcast_epidata.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions tests/testthat/test-model.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,29 @@ test_that("parse_data_frame warns when df contains fields not listed in meta", {
expect_no_warning(parse_data_frame(epidata_call, mock_df))
})

test_that("parse_data_frame warns when df contains int values with decimal component", {
epidata_call <- pub_flusurv(
locations = "ca",
epiweeks = 202001,
fetch_args = fetch_args_list(dry_run = TRUE)
)
# see generate_test_data.R
mock_df <- as.data.frame(readr::read_rds(testthat::test_path("data/flusurv-epiweeks.rds")))

# Int fields are returned as double
result <- parse_data_frame(epidata_call, mock_df)
expect_type(result$lag, "double")

# Replace int fields with decimal
mock_df$lag <- 4.3

# Warning when int values have a decimal component
expect_warning(
parse_data_frame(epidata_call, mock_df),
class = "epidatr__int_nonzero_decimal_digits"
)
})

test_that("parse_api_date accepts str and int input", {
expect_identical(parse_api_date("20200101"), as.Date("2020-01-01"))
expect_identical(parse_api_date(20200101), as.Date("2020-01-01"))
Expand Down