Skip to content
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.

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

match_name should allow loanbook_demo to have reserved columns sector and borderline #326

Closed
jdhoffa opened this issue Nov 25, 2020 · 1 comment · Fixed by #328
Closed

Comments

@jdhoffa
Copy link
Member

jdhoffa commented Nov 25, 2020

For the purpose of testing, we hope to use a stable loanbook_demo with the sector and borderline predefined, so that it does not depend on changes made in r2dii.data (especially to the classification files).

To do so, match_name must allow for sector and borderline to exist in the loanbook_demo input.

Relates to 2DegreesInvesting/r2dii.analysis#227

@maurolepore
Copy link
Contributor

The reprex below suggests may_add_sector_and_borderline() never meets the condition of the first if() statement:

# restructure_ald.R L75

may_add_sector_and_borderline <- function(data) {
  if (has_sector_and_borderline(data)) {
    data2 <- data
  } else {
    data2 <- add_sector_and_borderline(data)
  }

  data2
}
lbk <- dplyr::mutate(r2dii.data::loanbook_demo, sector = "a")
ald <- r2dii.data::ald_demo
r2dii.match::match_name(lbk, ald)
#> Error: `loanbook` can't have reserved columns:
#> sector

Created on 2020-11-25 by the reprex package (v0.3.0)

maurolepore added a commit that referenced this issue Nov 27, 2020
Closes #326
Closes #327 -- this is a simpler/safer way to solve it.
Relates to:
* https://github.com/2DegreesInvesting/r2dii.analysis/pull/227
* https://github.com/2DegreesInvesting/r2dii.analysis/pull/230

This change is internal, and motivated by our need to break the
dependency between r2dii.data and its reverse dependencies. The
problem we are trying to solve is this:

r2dii.data provides data that is used in some functions if its
reverse dependencies -- like r2dii.match. Concretely, changes in
the sector classification dataset (in r2dii.data) make tests to
fail in reverse dependencies like r2dii.match that store a
snapshot of expected output (e.g. of match_name()).

This commit allows us to use generate such snaphsots with a
frozen loanbook, and thus make r2dii packages less prone to
meaningless errors when CRAN checks for reverse dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants