Skip to content

BYOC_ID: 45966 Refactor sc demographics#1313

Draft
Jennit07 wants to merge 52 commits into
developmentfrom
refactor-sc-demog
Draft

BYOC_ID: 45966 Refactor sc demographics#1313
Jennit07 wants to merge 52 commits into
developmentfrom
refactor-sc-demog

Conversation

@Jennit07

@Jennit07 Jennit07 commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

PR not ready - WIP

  • data not ready in SDL views
  • dataset has dependencies.

Opening as a draft to review changes.

TODO:

  • review get_sc_demog_lookup_path() to work in byoc mode
  • Confirm SDL table name
  • confirm variables are as expected
  • Placeholder code for three dependency files -
    • All care home data,
    • spd_path = get_spd_path()
    • uk_pc_path = get_uk_postcode_path()
  • Search for all spd_path and update accordingly
  • Update targets
  • Test data with code

This PR is linked with SC Care Homes. Please merge care home branch into demographics branch before testing in development.

Data not ready in SDL - On hold for testing locally/in BYOC

#1299

@Jennit07 Jennit07 added On hold Waiting for something / someone outside of our control dependencies Pull requests that update a dependency file labels Apr 28, 2026
Comment on lines -40 to -49
if (!fs::file_exists(get_sandpit_extract_path(type = "demographics"))) {
sc_demog %>%
write_file(get_sandpit_extract_path(type = "demographics"),
group_id = 3206 # hscdiip owner
)

sc_demog %>%
process_tests_sc_sandpit(type = "demographics")
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've removed this intermediate saving of extracts. I think this would be redundant in the automation. However, if anyone has reasons to keep this, please let me know. I think this will also appear in other social care datasets!

Comment thread R/get_sc_lookup_paths.R
@Jennit07

Copy link
Copy Markdown
Collaborator Author

Is SC Demographics being added to dummy_targets and run_sdl or is this done in the SC Care Home refactoring?

Good spot - i think i still need to add this to dummy_targets and run_sdl

@Jennit07

Copy link
Copy Markdown
Collaborator Author

Is SC Demographics being added to dummy_targets and run_sdl or is this done in the SC Care Home refactoring?

Good spot - i think i still need to add this to dummy_targets and run_sdl

Change made. Ready for another review/test once we have the data. Thanks @LucyEmma22 for all your suggestions

Comment thread _targets.R
# Function
get_spd_path(),
get_spd_data(BYOC_MODE),
format = "file"

@LucyEmma22 LucyEmma22 Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be removed as spd_data is now a tibble rather than a filepath?

Comment thread _targets.R
get_slf_ch_name_lookup_path(),
slf_ch_name_lookup_data,
get_slf_ch_name_lookup_data(BYOC_MODE),
format = "file"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be removed as slf_ch_name_lookup_data is now a tibble rather than a filepath?

Comment thread _targets.R
slf_ch_name_lookup_path,
get_slf_ch_name_lookup_path(),
slf_ch_name_lookup_data,
get_slf_ch_name_lookup_data(BYOC_MODE),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be:

get_slf_ch_name_lookup_data( 
             denodo_connect = get_denodo_connection(BYOC_MODE = BYOC_MODE), 
             BYOC_MODE = BYOC_MODE
)

Comment thread _targets.R
spd_data,
# Function
get_spd_path(),
get_spd_data(BYOC_MODE),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be:

get_spd_data( 
             denodo_connect = get_denodo_connection(BYOC_MODE = BYOC_MODE), 
             BYOC_MODE = BYOC_MODE
)

Comment thread _targets.R
age = as.difftime(180, units = "days")
)
uk_postcode_data,
get_uk_postcode_data(BYOC_MODE)

@LucyEmma22 LucyEmma22 Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be:

get_uk_postcode_data( 
             denodo_connect = get_denodo_connection(BYOC_MODE = BYOC_MODE), 
             BYOC_MODE = BYOC_MODE
)

Comment thread SDL_process/run_sdl.r
"combined_deaths",
"chi_deaths"
)
types = c("demog")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be types = c("sc_demog_lookup")?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Aye. It should be sc_demog_lookup to match the current changes.

spd_data,
# Function
get_spd_data(BYOC_MODE),
format = "file"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be removed as spd_data is now a tibble rather than a filepath?

tar_target(
slf_ch_name_lookup_data,
get_slf_ch_name_lookup_data(BYOC_MODE),
format = "file"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be removed as slf_ch_name_lookup_data is now a tibble rather than a filepath?

# Target name
spd_data,
# Function
get_spd_data(BYOC_MODE),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be:

get_spd_data( 
             denodo_connect = get_denodo_connection(BYOC_MODE = BYOC_MODE), 
             BYOC_MODE = BYOC_MODE
)

tar_target(
# Target name
uk_postcode_data,
get_uk_postcode_data(BYOC_MODE)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be:

get_uk_postcode_data( 
             denodo_connect = get_denodo_connection(BYOC_MODE = BYOC_MODE), 
             BYOC_MODE = BYOC_MODE
)

# Care home name look up------
tar_target(
slf_ch_name_lookup_data,
get_slf_ch_name_lookup_data(BYOC_MODE),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be:

get_slf_ch_name_lookup_data( 
             denodo_connect = get_denodo_connection(BYOC_MODE = BYOC_MODE), 
             BYOC_MODE = BYOC_MODE
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the following targets also need to be added to this script?

source_gp_lookup
tests_source_gp_lookup
source_pc_lookup
tests_source_pc_lookup
all_care_home
tests_all_care_home

Comment thread R/fill_ch_names.R Outdated
Comment thread R/fill_ch_names.R Outdated
Comment thread R/fill_ch_names.R Outdated

# postcode data
spd_file <- read_file(spd_path) %>%
spd_file <- get_spd_data(BYOC_MODE) %>%

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This should be in the parameter now

Comment thread R/process_lookup_gpprac.R
spd_file <- read_file(
path = spd_path,
col_select = c(
spd_file <- get_spd_data(BYOC_MODE) %>%

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This should be in the parameter

Comment thread R/fill_ch_names.R
ch_name_lookup <- openxlsx::read.xlsx(ch_name_lookup_path,
detectDates = TRUE
) %>%
ch_name_lookup <- get_slf_ch_name_lookup_data(BYOC_MODE) %>%

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

add this to the parameter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants