Rename adjusted_scenario_demo output to target_corporate_economy

[jdhoffa]

George expressed (in a call ages ago) a preference to rename the "Adjusted Scenario Demo" output values to "Target Corporate Economy".

Methodologically, I think this sounds reasonable as they serve similar purposes. Curious what @jacobvjk thinks?

Also a question for @AlexAxthelm @MonikaFu , is this technically a breaking change? It affects the contents of value column of an output data-frame, but not the actual structure of the data-frame itself.

To me, that is still affecting the expected API of the package and should be dealt with cautiously, but curious what your feelings are there?

Supersedes #311

---

Brief description of the problem

library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
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>

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

# George wants here that `adjusted_scenario_demo` should be `target_corporate_economy`
out_sda %>% 
  distinct(emission_factor_metric)
#> # A tibble: 4 × 1
#>   emission_factor_metric
#>   <chr>                 
#> 1 projected             
#> 2 corporate_economy     
#> 3 target_demo           
#> 4 adjusted_scenario_demo

^{Created on 2024-02-06 with reprex v2.1.0}

AB#9904

[AlexAxthelm]

Smells like a definite version bump to me. My only question is if it's minor ( -> 0.4.0) or a patch (-> 0.3.1).

My inclination is that it's minor, since this is a breaking change (someone needs to rewrite code to get to the same behavior), but we're still not at the 1.0.0 release.

[jdhoffa]

Definitely minor!

I guess my question was more if this warrants following a "deprecation" process, wherein we offer one release on CRAN with a warning about it's future deprecation. Or if we just deprecate it and explain.

[AlexAxthelm]

I guess my question was more if this warrants following a "deprecation" process, wherein we offer one release on CRAN with a warning about it's future deprecation. Or if we just deprecate it and explain.

Hmm. this doesn't seem to fit in the lifecycle paradigm, since it's not adding/removing function or an argument, just a behavior change.

@jdhoffa if we were to support a formal change (with time to adjust), is that even possible? That would change the structure of the data.frame (add rows, not just changing contents).

One possibility for this type of deprecation is to add an argument (born deprecated 😢 ) that is default false, but cazn be used to restore the old behavior (to not block users until they can update their code)

target_sda <- function(
  blah,
  blah,
  blah,
  use_old_names = FALSE
) {
  if (use_old_names){
    deprecate_warn("use_old_names argument is being removed in next release")
  } else {
    #whatever magic is needed to only show the warning the first time
    message("r2dii.analysis 0.4.0 changed the values for emission_factor_metric. Here be dragons, etc")
  }
  #normal function body
}

[jdhoffa]

Hmm that seems like an awkward but appropriate solution, given that this code is very much already in the wild.

Tangent for future @jdhoffa: What do you think would have been best practice here in terms of design principles?

• To ensure output data was tidy, we preferred to output long-form data, with columns metric and value
• The result of that is that we codify a design choice in the values of the output data-frame (here the name adjusted_scenario_demo which we made up and is not directly in the input data)

Asking as a potential design consideration for any future "metric calculating" packages we may wish to create.

[AlexAxthelm]

I don't think there were any wrong choices made along the way. Everything seems reasonable so far, and this is just a normal (but weird) deprecation process.

I think the weirdness comes in that we're using code to define the contents of a table, rather than letting that live in a data dictionary of some kind (though I don't know of a good way to handle deprecations in those either).

[jacobvjk]

I am generally on board with this change, although I think it is still not ideal. The new label would not have any reference to which scenario is being used, although the target_corporate_economy trajectory would vary depending on the target_scenario used on the portfolio. I.e. If we use nze_2050 instead of demo, perviously we would have gotten:

• target_nze_2050
• adjusted_scenario_nze_2050

Now we would get:

• target_nze_2050
• target_corporate_economy

... where target_corporate_economy has the same label as if we had run the calculation on the demo scenario, but it will have different values.

The consistent, albeit clunky solution would be to use target_corporate_economy_xy

[jacobvjk]

maybe it would be easier to just drop the target_ prefix altogether. we would then get something like

• nze_2050
• corporate_economy_nze_2050

still not ideal, just throwing out some ideas

[jdhoffa]

Mainly I para-phrased George in creating this issue as I was doing some spring-cleaning... so I think it best to discuss it with him? I don't have a lot of strong opinions here.

[jdhoffa]

although my two cents:

• generally, I don't mind "clunky but accurate" so I think target_corporate_economy_demo sounds reasonable
• at the same time, I am hesitant to include the term corporate_economy in MORE places, as it is a horribly poorly defined terminology, and not often clear to most people what it is actually talking about...

[MonikaFu]

Do I understand correctly that this change would only affect the demo dataset? Or is it also somehow interwoven in r2dii.analysis when called on real data? It sounds like a change I'd need to adjust for in r2dii.plot if I'd want it to run with r2dii.analysis outputs so it's somewhat breaking I'd say but because it is changing the values and not variables (inside the output table vs. design of the output table) I'd say it is a minor change.

My 2 cents about naming. Why don't we call it benchmark instead of corporate_economy and why would we then not rename adjusted_scenario_demo to target_benchmark? If I understand correctly - target_demo is to projected as adjusted_scenario_demois tocorporate_economy`? Then my proposal would be:

projected -- target_demo
benchmark -- target_benchmark_demo

Although this doesn't solve all ambiguity issues I guess..

[jdhoffa]

Yes absolutely a breaking change (I tagged the issue as such), and yes absolutely has implications for r2dii.plot!

Regarding the conversation, I think anyone with an opinion here should sit down on a call with George (not urgently, just eventually). I am not particularly picky what the values are, and the flag to change it in the first place came from him, so that will likely be the most efficient way to come to a decision :-)

Labeled as "help wanted" until we get input and confirmation from George.