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

CU-86964zm4d fix preprocessing #496

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Conversation

mart-r
Copy link
Collaborator

@mart-r mart-r commented Oct 10, 2024

Since #469 there's been a bit of an issue with preprocessing. Namely, it wasn't ignoring UKClinicalRefsetsRF2 as it should have done.

I have made the necessary change.

The other small QoL change this PR introduces is the OPCS4 refset ID. Since end of 2023 the refset ID for OCS4 mappings is different. But the default had been the old ID. This PR reversed that logic and has the new refset ID as a default rather than the old one and changes to the old one if/when needed.

And the third thing that was problematic was checking of extension (e.g UK/UK Drug) outside the loop since extensions are now handled automatically and can change while iterating over them in a bundle.

To ensure this all works exactly as it should I ran a test on Snomed UK Clinical Edition and Drug Extension releases with both the old version (i.e one available with medcat 1.12) and this version. And the results were identical in terms of:

  • Output of Snomed.to_concept_df
  • Output of Snomed.map_snomed2icd10
  • Output of Snomed.map_snomed2opcs4
  • Output of Snomed.relationship2json with relationship code "116680003" (ISA relationships)

@tomolopolis
Copy link
Member

Task linked: CU-86964zm4d Fix preprocessing issues

Fix the default value being tested for (i.e in case of international release that'll be shown).
Add a test for old UK extension.
I.e determinie if a UK release/bundle is used for OPCS4/ICD10 mappings splitting.
Always returning separate refsets for ICD10 and OSC internally, even if the latter is None.
Copy link
Member

@tomolopolis tomolopolis left a comment

Choose a reason for hiding this comment

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

lgtm - once the model creation code base is more mature, might make sense to move this module into that repo.

@mart-r mart-r merged commit e924798 into master Nov 1, 2024
8 checks passed
@mart-r mart-r deleted the CU-86964zm4d-fix-preprocessing branch November 18, 2024 16:22
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.

2 participants