Map unknown concepts to 0 rather than null. When mapping from known c… #279
Conversation
…oncepts to standard concepts, map to 0 if no results are found, but continue to map to nothing if a concept is found but it resides in the wrong domain.
There was a problem hiding this comment.
Pull Request Overview
This PR renames ConceptResolver to StandardConceptResolver and updates the return behavior for concept lookups to return an unknown concept ID (0) instead of null when mappings are not found. Additionally, SQL queries in documentation are standardized, and deduplication logic is enhanced to handle records with unknown concept IDs.
- Renamed
ConceptResolvertoStandardConceptResolverthroughout the codebase - Changed
ConceptLookup.GetConceptCode()return type fromint?toint, returning 0 for unknown concepts - Enhanced deduplication logic in recorder classes to differentiate records with unknown concept IDs (0) by comparing source values
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| StandardConceptResolver.cs | Renamed class from ConceptResolver and updated logic to return unknown concept ID (0) when mappings fail |
| ConceptLookup.cs | Changed return type from nullable to non-nullable int, returning 0 for unknown concepts |
| RecordTransformer.cs | Updated to use renamed StandardConceptResolver |
| Multiple selector files | Updated constructor parameters to use StandardConceptResolver |
| Multiple transformer files | Updated constructor parameters and field types to use StandardConceptResolver |
| Program.cs | Updated service registration for renamed class |
| Multiple recorder files | Added deduplication checks for records with concept_id = 0 |
| Documentation files | Standardized SQL query formatting (replaced top 1 with limit 1, renamed PatientId to NhsNumber, changed table alias) |
Comments suppressed due to low confidence (2)
OmopTransformer/StandardConceptResolver.cs:132
- [nitpick] Corrected spelling of 'empty' to 'empty array' for clarity, as the method returns an array.
OmopTransformer/StandardConceptResolver.cs:134 - The expression 'A == false' can be simplified to '!A'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| and co.person_id = p.person_id | ||
| and co.RecordConnectionIdentifier = r.RecordConnectionIdentifier | ||
| and co.condition_concept_id = r.condition_concept_id | ||
| and (co.condition_concept_id != 0 or and co.condition_source_value = r.condition_source_value) |
There was a problem hiding this comment.
Syntax error in SQL: duplicate 'or and' keywords. Should be either 'or' or 'and', not both. The correct syntax should be and (co.condition_concept_id != 0 or co.condition_source_value = r.condition_source_value).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| and co.person_id = p.person_id | ||
| and co.RecordConnectionIdentifier = r.RecordConnectionIdentifier | ||
| and co.condition_concept_id = r.condition_concept_id | ||
| and (co.condition_concept_id != 0 or and co.condition_source_value = r.condition_source_value) |
There was a problem hiding this comment.
Syntax error in SQL: duplicate 'or and' keywords. Should be either 'or' or 'and', not both. The correct syntax should be and (co.condition_concept_id != 0 or co.condition_source_value = r.condition_source_value).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@james-cockayne I've opened a new pull request, #280, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@james-cockayne I've opened a new pull request, #281, to work on those changes. Once the pull request is ready, I'll request review from you. |
[WIP] WIP address feedback to map unknown concepts to 0
[WIP] Update mapping of unknown concepts to 0 instead of null
…nswerdigital/oxford-omop-data-mapper into feature/map_to_unknown_concept
…standard concept.
…nswerdigital/oxford-omop-data-mapper into feature/map_to_unknown_concept
…oncepts to standard concepts, map to 0 if no results are found, but continue to map to nothing if a concept is found but it resides in the wrong domain.