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

Align naming of metric-like columns for output of target_* functions #469

Closed
jdhoffa opened this issue Feb 6, 2024 · 4 comments
Closed
Labels
ADO Maintenance Day! breaking change ☠️ API change likely to affect existing code feature a feature request or enhancement small Likely finished in under a day

Comments

@jdhoffa
Copy link
Member

jdhoffa commented Feb 6, 2024

Current behaviour

target_market_share() outputs a column metric
target_sda() outputs a column emission_factor_metric

Desired behaviour

Align so that both columns either output:

  • production_metric and emission_factor_metric
  • both metric

Supersedes #311


Brief description of the problem

library(r2dii.data)
library(r2dii.match)
library(r2dii.analysis)

matched <- loanbook_demo %>%
  match_name(abcd_demo) %>%
  prioritize()

# Calculate targets at portfolio level
matched %>%
  target_market_share(
    abcd = abcd_demo,
    scenario = scenario_demo_2020,
    region_isos = region_isos_demo
    )

#> # A tibble: 792 × 10
#>    sector     technology  year region scenario_source metric     production
#>    <chr>      <chr>      <int> <chr>  <chr>           <chr>           <dbl>
#>  1 automotive electric    2020 global demo_2020       projected     324592.
#>  2 automotive electric    2020 global demo_2020       target_cps    324592.
#>  3 automotive electric    2020 global demo_2020       target_sds    324592.
#>  4 automotive electric    2020 global demo_2020       target_sps    324592.
#>  5 automotive electric    2021 global demo_2020       projected     339656.
#>  6 automotive electric    2021 global demo_2020       target_cps    329191.
#>  7 automotive electric    2021 global demo_2020       target_sds    352505.
#>  8 automotive electric    2021 global demo_2020       target_sps    330435.
#>  9 automotive electric    2022 global demo_2020       projected     354720.
#> 10 automotive electric    2022 global demo_2020       target_cps    333693.
#> # ℹ 782 more rows
#> # ℹ 3 more variables: technology_share <dbl>, scope <chr>,
#> #   percentage_of_initial_production_by_scope <dbl>

# Calculate targets at company level
out_market_share <- matched %>%
  target_market_share(
  abcd = abcd_demo,
  scenario = scenario_demo_2020,
  region_isos = region_isos_demo
  ) 

out_sda <- matched %>% 
  target_sda(
    abcd = abcd_demo,
    co2_intensity_scenario = co2_intensity_scenario_demo,
    region_isos = region_isos_demo
  )
#> Warning: Removing rows in abcd where `emission_factor` is NA

# note that the column names of the metrics are different
all_names <- c(names(out_sda), names(out_market_share))
all_names[grep("metric", all_names)]
#> [1] "emission_factor_metric" "metric"

Created on 2024-02-06 with reprex v2.1.0

AB#9902

@jdhoffa jdhoffa added small Likely finished in under a day feature a feature request or enhancement breaking change ☠️ API change likely to affect existing code labels Feb 6, 2024
@jdhoffa
Copy link
Member Author

jdhoffa commented Feb 6, 2024

See also #313 for some useful discussion.

@jdhoffa jdhoffa added the ADO Maintenance Day! label Feb 6, 2024
@jdhoffa jdhoffa self-assigned this Feb 6, 2024
@jdhoffa jdhoffa added ADO Maintenance Day! and removed ADO Maintenance Day! labels Feb 19, 2024
@jdhoffa jdhoffa removed their assignment Apr 10, 2024
@cjyetman
Copy link
Member

@jdhoffa and @jacobvjk noting that the documentation of the output of these functions in pacta.loanbook somewhat increases the urgency of deciding whether or not to rename these columns

@jacobvjk
Copy link
Member

While I agree that the naming choices here are far from optimal, changing these will lead to downstream issues at the very lest in pacta.multi.loanbook and any other workflow we have built for supervisors which depend on the given variable names. We could update all of them, but is it really worth it? This has been around for years and does not seem to have generated confused feedback from users.

@jdhoffa
Copy link
Member Author

jdhoffa commented Feb 11, 2025

Happy to close this without action

@jdhoffa jdhoffa closed this as completed Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADO Maintenance Day! breaking change ☠️ API change likely to affect existing code feature a feature request or enhancement small Likely finished in under a day
Projects
None yet
Development

No branches or pull requests

3 participants