Skip to content

Conversation

@rxu17
Copy link
Contributor

@rxu17 rxu17 commented Feb 2, 2025

Purpose: Currently there are known invalid ONCOTREE_CODE values in the cohort's clinical file (for certain sites). This will be fixed upstream in the processing pipeline. Meanwhile the original ticket has been repurposed to throw a warning explicitly stating which codes are invalid prior to the code erroring due to said invalid codes (because we should be catching the invalid codes upstream but while we don't have that automation in place, we will have this here as a backup).

See GEN-1717 for more BG info on this.
Testing: Tested on RENAL cohort in staging pipeline which has the invalid codes in the uploads and threw the expected warning:
image

@rxu17 rxu17 requested a review from a team as a code owner February 2, 2025 22:57
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2025

ids=["all_invalid", "some_invalid"],
)
def test_that_check_oncotree_codes_gives_expected_warning_when_invalid_codes(
caplog, input_data, oncotree_dict, expected_warning
Copy link
Member

@thomasyu888 thomasyu888 Feb 3, 2025

Choose a reason for hiding this comment

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

TIL: caplog! Is that a pytest thing?

Seems like a cool thing to post about in #dpe-topics

Copy link
Contributor Author

@rxu17 rxu17 Feb 3, 2025

Choose a reason for hiding this comment

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

Yup it is: https://docs.pytest.org/en/6.2.x/logging.html#caplog-fixture. I think we used this in recover, (a little in ) main genie (but mainly because we have our own logging there) and I saw it in the synapse python client.

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 LGTM!

@rxu17 rxu17 merged commit 12467d5 into develop Feb 6, 2025
4 checks passed
@rxu17 rxu17 deleted the gen-1717-consolidate-codes branch February 6, 2025 05:16
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.

3 participants