Skip to content
This repository has been archived by the owner on Oct 30, 2024. It is now read-only.

Refactor and test company aggregation for tms sectors #75

Merged
merged 7 commits into from
May 5, 2023

Conversation

jacobvjk
Copy link
Member

@jacobvjk jacobvjk commented May 4, 2023

relates to #46

This PR:

  • refactors calculate_company_aggregate_alignment_tms() and splits some functionality out into sub functions
  • adds unit tests

Note:

  • the remaining functions will get refactored and unit tested in separate PRs to keep the PR size in check

@jacobvjk jacobvjk changed the title Test company aggregation Refactor and test company aggregation for tms sectors May 4, 2023
@jacobvjk jacobvjk marked this pull request as ready for review May 4, 2023 16:51
add_total_deviation() %>%
calculate_company_alignment_metric(scenario = scenario) %>%
dplyr::arrange(
.data$group_id, .data$sector, .data$name_abcd, .data$region, .data$year
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know @AlexAxthelm strongly prefers lists of arguments like this to be one per line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean this?

dplyr::arrange(
   .data$group_id,
   .data$sector,
   .data$name_abcd,
   .data$region,
   .data$year
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@jacobvjk jacobvjk merged commit 07f696f into main May 5, 2023
@jacobvjk jacobvjk deleted the test-company-aggregation branch May 5, 2023 07:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants