Skip to content

BYOC_ID: 45986 Refactor PIS#1326

Open
LucyEmma22 wants to merge 14 commits into
developmentfrom
refactor-pis
Open

BYOC_ID: 45986 Refactor PIS#1326
LucyEmma22 wants to merge 14 commits into
developmentfrom
refactor-pis

Conversation

@LucyEmma22

@LucyEmma22 LucyEmma22 commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Complete and reviewed. Waiting for PIS data.

TO-DO:

  • Discuss refactoring of get_it_prescribing_path. It has been refactored like get_it_deaths_path but this currently returns a dummy path if BYOC_MODE = TRUE which differs from get_spd_path.
  • Check SDL table name
  • Check whether year should be converted to c_year
  • Check year column name
  • Test locally

@LucyEmma22 LucyEmma22 added On hold Waiting for something / someone outside of our control BYOC 1a: Ready for Review 1 Refactoring completed by author (without data) 1: Preprod Data Not Available Denodo data not available in preprod labels May 12, 2026
@lizihao-anu
lizihao-anu self-requested a review May 19, 2026 08:41
@lizihao-anu lizihao-anu self-assigned this May 26, 2026
@Jennit07
Jennit07 self-requested a review May 26, 2026 09:00
@LucyEmma22
LucyEmma22 removed the request for review from lizihao-anu May 26, 2026 09:00
Comment thread R/get_it_extract_paths.R Outdated
Comment on lines +79 to +100
get_it_prescribing_path <- function(year, it_reference = NULL, BYOC_MODE, ...) {
if (isTRUE(BYOC_MODE)) {
return("dummy_BYOC_it_prescribing_path")
} else {
it_reference <- check_it_reference(it_reference)
if (is.null(it_reference)) {
it_pis_path <- get_file_path(
directory = fs::path(get_slf_dir(), "IT_extracts/anon-chi-IT"),
file_name_regexp = stringr::str_glue(
"anon-SCTASK[0-9]{{7}}_PIS_{convert_fyyear_to_year(year)}.parquet"
)
)
} else {
it_reference <- check_it_reference(it_reference)

it_pis_path <- get_file_path(
directory = fs::path(get_slf_dir(), "IT_extracts/anon-chi-IT"),
file_name = stringr::str_glue(
"anon-SCTASK{it_reference}_PIS_{convert_fyyear_to_year(year)}.parquet"
it_pis_path <- get_file_path(
directory = fs::path(get_slf_dir(), "IT_extracts/anon-chi-IT"),
file_name = stringr::str_glue(
"anon-SCTASK{it_reference}_PIS_{convert_fyyear_to_year(year)}.parquet"
)
)
)
}
return(it_pis_path)

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.

Did you get inspiration from the IT CHI deaths here?

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.

sorry getting confused with SPD file paths!

# Format prescribing
dplyr::mutate(
dob = lubridate::dmy(.data$dob)
dob = lubridate::as_date(.data$dob)

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.

good spot!

@Jennit07 Jennit07 left a comment

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 need to come back to this PR to test when the data is available. It looks okay but im a little confused with how to handle the IT file paths. It looks like this was inspired by the CHI deaths file paths which has previously passed the tests with NSS. But my first thought was to handle this similar to the SPD file paths. This would maintain the consistency in the code. Happy to discuss/do further testing on this

@Jennit07 Jennit07 added ready for PHS testing 1b: Peer Review 1 Complete First peer review completed by reviewer (without data) and removed 1a: Ready for Review 1 Refactoring completed by author (without data) labels Jun 16, 2026
@LucyEmma22

LucyEmma22 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

I need to come back to this PR to test when the data is available. It looks okay but im a little confused with how to handle the IT file paths. It looks like this was inspired by the CHI deaths file paths which has previously passed the tests with NSS. But my first thought was to handle this similar to the SPD file paths. This would maintain the consistency in the code. Happy to discuss/do further testing on this

@Jennit07 Yes I looked at CHI deaths and did the same thing here. The only change is that it returns "dummy_BYOC_it_prescribing_path" when in BYOC_MODE. Happy to discuss though as I agree that consistency would be good.

@LucyEmma22 LucyEmma22 removed On hold Waiting for something / someone outside of our control 1b: Peer Review 1 Complete First peer review completed by reviewer (without data) labels Jun 30, 2026
Comment thread R/get_it_extract_paths.R Outdated
)
get_it_prescribing_path <- function(year, it_reference = NULL, BYOC_MODE, ...) {
if (isTRUE(BYOC_MODE)) {
return("dummy_BYOC_it_prescribing_path")

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.

can use get_dummy_file_path().

@LucyEmma22 LucyEmma22 added 1b: Peer Review 1 Complete First peer review completed by reviewer (without data) On hold Waiting for something / someone outside of our control and removed 1b: Ready for PHS Testing labels Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1b: Peer Review 1 Complete First peer review completed by reviewer (without data) 1: Preprod Data Not Available Denodo data not available in preprod BYOC On hold Waiting for something / someone outside of our control

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants