Skip to content

Commit

Permalink
Fill NAs with zero in abcd (#460)
Browse files Browse the repository at this point in the history
* Add failing test

* Add util to fill NAs instead of filter

* update NEWS.md

* add issue number to test

* expect new behaviour

* Solve failing test

* Fix NEWS.md
  • Loading branch information
jdhoffa authored Feb 5, 2024
1 parent 73e1b15 commit 1f7d3d8
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 3 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# r2dii.analysis (development version)

* `target_market_share` now handles `abcd` with rows where `production` is `NA` by filling with `0` (#423).

# r2dii.analysis 0.3.0

* `target_sda` now uses final year of scenario as convergence target when `by_company = TRUE` (#445).
Expand Down
2 changes: 1 addition & 1 deletion R/target_market_share.R
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ target_market_share <- function(data,

data <- rename_and_warn_ald_names(data)

abcd <- filter_and_warn_na(abcd, "production")
abcd <- fill_and_warn_na(abcd, "production")

region_isos <- change_to_lowercase_and_warn(region_isos, "isos")

Expand Down
14 changes: 14 additions & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ filter_and_warn_na <- function(data, column) {
return(data)
}

fill_and_warn_na <- function(data, column) {
if (anyNA(data[[column]])) {
name_dataset <- deparse(substitute(data))
warning_message = paste("Filling in rows of", name_dataset, "where `{column}` is NA with 0")
warn(
glue(warning_message),
class = "fill_nas_crucial_economic_input"
)

data[[column]] <- tidyr::replace_na(data[[column]], 0)
}
return(data)
}

warn_grouped <- function(data, message) {
if (dplyr::is_grouped_df(data)) warn(message)

Expand Down
22 changes: 20 additions & 2 deletions tests/testthat/test-target_market_share.R
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ test_that("w/ NAs in crucial columns, errors with informative message", {
expect_error_crucial_NAs_scenario("smsp")
})

test_that("filters and warns when input-data has NAs", {
test_that("fills and warns when input-data has NAs", {
matched <- fake_matched()
abcd <- fake_abcd(production = c(1, NA))
scenario <- fake_scenario()
Expand All @@ -180,7 +180,7 @@ test_that("filters and warns when input-data has NAs", {
abcd,
scenario,
region_isos_stable),
class = "na_crucial_economic_input")
class = "fill_nas_crucial_economic_input")
})

test_that("outputs expected names", {
Expand Down Expand Up @@ -1410,3 +1410,21 @@ test_that("region_isos only has lowercase isos #398", {
)
)
})

test_that("with `abcd` with `NA` for start year, replaces `NA` with 0 (#423)", {
expect_warning(
out <- target_market_share(
fake_matched(),
fake_abcd(year = c(2020, 2025), production = c(NA_real_, 1)),
fake_scenario(year = c(2020, 2025)),
region_isos_stable
),
class = "fill_nas_crucial_economic_input"
)

out <- out %>%
filter(metric == "projected") %>%
arrange(year)

expect_equal(out$production, c(0, 1))
})

0 comments on commit 1f7d3d8

Please sign in to comment.