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

Stage1 data cleaning #80

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Stage1 data cleaning #80

wants to merge 32 commits into from

Conversation

ZoeMZou
Copy link
Contributor

@ZoeMZou ZoeMZou commented Jan 16, 2025

PR: Refinements for Script Structure & Data Processing

(Based on discussions on 28th January 2025)

🔹 1. Structural Changes
📌 1.1 Naming Conventions

  • Removed _stage1 from scripts to improve clarity and consistency.

  • The old stage1_data_cleaning.R script was handling:

    1. Setting reference levels for categorical variables
    2. Applying quality assurance
    3. Applying inclusion/exclusion criteria
  • The new structure separates these tasks into three dedicated functions:

    • fn-ref.R → Sets reference levels
    • fn-qa.R → Applies quality assurance
    • fn-inex.R→ Applies inclusion/exclusion criteria
  • The main script data_cleaning.R calls these functions, ensuring: Project-specific edits are made in function scripts and data_cleaning.R remains clear and workflow-focused.

📌 1.2 Output Dataset Naming

  • The cleaned dataset output is now: input_{cohort}.rds

  • The preprocessed dataset output is now:
    input_{cohort}.rds → input_{cohort}_0.rds (to avoid YAML conflicts)

🔹 2. fn-ref.R — Reference Level Settings

📌 2.1 Data Formatting Fixes

  • Some categorical variables were not factors, and some numeric variables were not numeric. I added code back to preprocess_data.R to ensure correct formatting:
    🔗 See fix in preprocess_data.R

📌 2.2 Code Deletions from Old Repo
Index Date: Already correct—no correction needed.
🔗 Old repo reference
Deprivation (IMD): No need for re-categorization; new repo includes IMD (1–5) directly.
🔗 Old repo reference

📌 2.3 Sex Category Adjustments
Old repo: Only male (M) and female (F).
New repo: Four levels → female, male, intersex, unknown.
Fix:
Set non-male/non-female values to missing. Retain three levels: female, male, unknown.

🔹 3. fn-qa.R — Quality Assurance Fixes

📌 3.1 Fixing "Date of Death" Message
The original message incorrectly stated that NA values were being removed.
Fix: Message now correctly states:
"Quality assurance: Date of death is invalid (on or before 1/1/1900 or after current date)"

🔗 Old repo reference

📌 3.2 Fixing Missing Data Handling
Issue: Patients with missing sex were marked as missing for the entire record, which was unintended.
Fix: The new script preserves records and only marks sex as missing.
🔗 Updated QA code

📌 3.3 Update to qa_bin_prostate_cancer
Issue:
The current method introduced missing values due to logic operations with NA values. NA values were assigned to alive patients who had no recorded prostate cancer diagnosis but were affected by the logic.
The check for cause of death due to prostate cancer was unnecessary. Patients who died from prostate cancer before the index date are already excluded later in the study. Removing this check ensures all alive participants receive a valid classification (TRUE/FALSE) rather than NA.
🔗 Updated method

🔹 4. fn-inex.R — Inclusion/Exclusion Criteria Fixes

📌 4.1 Code Corrections
The new repo includes inex_bin_alive, so we can use it directly instead of recalculating.
🔗 Old repo reference
🔗 New implementation

📌 4.2 Variable Renaming
death_date → cens_date_death
has_follow_up_previous_6monthsinex_bin_6m_reg
deregistration_datecens_date_dereg

📌 4.3 Removal of Unnecessary Code
Issue: The old repo had an unnecessary step for active registration at index.
Fix:
Removed redundant filtering for active registration, as inex_bin_6m_reg already ensures this. I rewrote the print message to reflect this.
🔗 Old repo reference
🔗 Updated function

📌 4.4 Fix for Exclusion Criteria in the Vax Cohort
Issue: Errors in the vax cohort exclusion criteria due to incorrect variable types.
Fix:
Redefined data types to ensure they are numeric before calculating vax_mixed.
🔗 Updated fix

🔹 5. Other Updates

📌 5.1 Cause of Death Extraction Simplified
A more efficient method for extracting cause of death has become available in OpenSAFELY.
Fix: Updated our code to align with the latest OpenSAFELY documentation.
🔗 Updated function

ZoeMZou added 10 commits January 7, 2025 16:56
Further consider secondary care diagnosis
1. remove opa diagnosis for hospitable admission.
2. revise covid-19 severity variable by focusing on primary diagnosis only.
To include diagnosis in any position in the function. Previously we just included primary diagnosis and first code of secondary diagnosis.

The definition of Covid-19 hospitalisation `sub_date_covid19_hospital` did not use any created function as its definition is very unique and not worth creating a function for it self.
@ZoeMZou ZoeMZou linked an issue Jan 16, 2025 that may be closed by this pull request
@ZoeMZou ZoeMZou requested review from venexia and removed request for venexia January 30, 2025 12:14
@ZoeMZou
Copy link
Contributor Author

ZoeMZou commented Jan 30, 2025

Hi @venexia,

This PR is now ready for your review. It runs successfully locally. Since the revisions in this script are quite detailed, I’ve listed the key changes at the beginning of the PR to make it easier to navigate and review.

Thank you very much.

Best wishes,
Zoe

@ZoeMZou ZoeMZou marked this pull request as ready for review January 30, 2025 18:48
Copy link
Contributor

@venexia venexia left a comment

Choose a reason for hiding this comment

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

Hi @ZoeMZou. Great work - well done! Just a few things to address:

  • Some of the QA criteria are out of date - these should be updated to match the current protocol
  • Some of the code for QA and inex could be simplified by using dplyr::case_when and/or removing double negatives
  • The naming structure for the files through the pipeline is not very intuitive - we should agree an approach for this when we next meet the post-covid-events team
  • The currently generated dummy data does not have the variation to test this code properly - we should resolve this so we can double check we are removing the correct people (I did this for the first QA criterion but haven't gone through each criteria in this review)
  • The formatting of the scripts is inconsistent and should be reviewed - we can automate this using linting but I need to look up how to set it up

consort[nrow(consort)+1,] <- c("Quality assurance: Year of birth is before 1793 or year of birth exceeds current date",
nrow(input))

print('Quality assurance: Date of death is invalid (on or before 1/1/1900 or after current date)')
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this something like:

print('Quality assurance: Date of death is invalid (on or before earliest expected date or after current date)')

It is only on or before 1/1/1900 when study_dates$earliest_expec=1/1/1900.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though on further reading of the protocol, I notice this criteria is listed as 'Remove individuals whose date of death is after today' and does not mention invalid death dates.

Copy link
Contributor Author

@ZoeMZou ZoeMZou Feb 10, 2025

Choose a reason for hiding this comment

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

Decision from today's meeting (February 11, 2025): The repository will be revised to ensure consistency with the protocol regarding inclusion criteria and quality assurance.

consort[nrow(consort)+1,] <- c("Quality assurance: Date of death is invalid (on or before 1/1/1900 or after current date)",
nrow(input))

print('Quality assurance: Pregnancy/birth codes for men')
Copy link
Contributor

Choose a reason for hiding this comment

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

I am in two minds about including | is.na(input$cov_cat_sex) - the people with missing sex might include, for example, pregnancy/birth codes for men. This applies to all sex specific QA criteria.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, individuals with missing sex should be included here. Only pregnancy/birth codes for men (HRT or COCP meds for men; prostate cancer codes for women) are excluded. We will handle the exclusion of individuals with missing sex in fn-inex.R to ensure accurate counts. Excluding them here would lead to inconsistencies in the reported number of missing sex cases.

analysis/data_cleaning/data_cleaning.R Show resolved Hide resolved
consort[nrow(consort)+1,] <- c("Inclusion criteria: Did not recieve a second dose vaccination before their first dose vaccination",
nrow(input))

print('Inclusion criteria: Did not recieve a mixed vaccine products before 07-05-2021')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could make this more readable using dplyr::case_when().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will use this PR to merge preprocess and data_cleaning for better efficiency. Key steps to follow:

  • Apply Inclusion Criteria First: Ensure inclusion criteria are applied before quality assurance checks.

  • Improve Readability: Use case_when for filtering datasets to enhance clarity.

  • Step-by-Step Validation: Check the dataset at each stage to confirm that the code is functioning as expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should discuss whether the dummy data is appropriate to test these scripts when we next meet. I have everyone born in the year 1975 in my dataset so nobody matches the first QA criterion and it is hard to tell if it is working without modifying the dummy data (which we could do).

@@ -112,7 +114,7 @@ df1[,colnames(df)[grepl("tmp_",colnames(df))]] <- NULL

# Save input -------------------------------------------------------------------

saveRDS(df1, file = paste0("output/input_",cohort_name,".rds"), compress = TRUE)
saveRDS(df1, file = paste0("output/input_",cohort_name,"_0.rds"), compress = TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended as a permanent change or was it for testing purposes? The life course of the file is now input_cohort.csv.gz > input_cohort_0.rds > input_cohort.rds, which is a little confusing as you expect the first and third file to be the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decision from today's meeting (11th Feb 2025):

  • We will not create an intermediate dataset from preprocess.

  • After merging preprocess and data_cleaning, the only output will be input_cohort_clean.rds.

analysis/dataset_definition/codelists.py Show resolved Hide resolved
analysis/dataset_definition/codelists.py Show resolved Hide resolved
analysis/dataset_definition/codelists.py Show resolved Hide resolved
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.

Stage_1_data_cleaning issues
2 participants