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

All *profile*() functions now warn if the user adjusts the *threshold arguments in any way #754

Closed
wants to merge 4 commits into from

Conversation

maurolepore
Copy link
Contributor

@maurolepore maurolepore commented Apr 2, 2024

Closes #753

This reduces the chances that the user will modify the thresholds by mistake.

NOTE. I see a failing check. It may be unrelated but I need to investigate.

reprex
devtools::load_all()
#> ℹ Loading tiltIndicator

companies <- example_companies()
scenarios <- example_scenarios(year = 2050)

# Using the default thresholds yields no warning
out <- sector_profile(companies, scenarios)

# Using the `*threshold` argument in any way yields a warning
out <- sector_profile(companies, scenarios, low_threshold = 1/4)
#> Warning: The default thresholds are generally the most useful.
#> ℹ Do you really need to adjust them?

out <- sector_profile(companies, scenarios, high_threshold = 1/4)
#> Warning: The default thresholds are generally the most useful.
#> ℹ Do you really need to adjust them?

# You get the warning for all indicators, and even if you use the same value
# as the default -- which isn't ideal but is simplest
formals(emissions_profile)
#> $companies
#> 
#> 
#> $co2
#> 
#> 
#> $low_threshold
#> 1/3
#> 
#> $high_threshold
#> 2/3

co2 <- example_products()
out <- emissions_profile(companies, co2, low_threshold = 1/3)
#> Warning: The default thresholds are generally the most useful.
#> ℹ Do you really need to adjust them?

TODO

  • Link related issue/PR.
  • Describe the goal of the PR. Avoid details that are clear in the diff.
  • Mark the PR as draft.
  • Include a unit test.
  • Review your own PR in "Files changed".
  • Ensure the PR branch is updated.
  • Ensure the checks pass.
  • Change the status from draft to ready.
  • Polish the PR title and description.
  • Assign a reviewer.

EXCEPTIONS

  • Slide here any item that you intentionally choose to not do.

@maurolepore maurolepore force-pushed the 751_warn-custom-thresholds branch from ccbce19 to 32240f8 Compare April 2, 2024 23:56
@maurolepore maurolepore mentioned this pull request Apr 2, 2024
11 tasks
@maurolepore maurolepore requested a review from Tilmon April 2, 2024 23:58
@maurolepore maurolepore changed the title Check for non missing threshold All *profile*() functions now warn if the user adjusts the *threshold arguments in any way Apr 2, 2024
@maurolepore maurolepore marked this pull request as ready for review April 3, 2024 00:01
@maurolepore maurolepore marked this pull request as draft April 3, 2024 00:02
@maurolepore maurolepore removed the request for review from Tilmon April 3, 2024 00:02
@maurolepore
Copy link
Contributor Author

Closing in favor of 2DegreesInvesting/tiltWorkflows#146

@maurolepore maurolepore closed this Apr 3, 2024
@maurolepore maurolepore deleted the 751_warn-custom-thresholds branch April 3, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant