Gender checks that use descendant concepts + meas unit pairs cleared #524
Conversation
to do: update the documentation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #524 +/- ##
===========================================
- Coverage 86.48% 85.00% -1.49%
===========================================
Files 16 16
Lines 888 907 +19
===========================================
+ Hits 768 771 +3
- Misses 120 136 +16 ☔ View full report in Codecov by Sentry. |
| FIELD,plausibleAfterBirth,"The number and percent of records with a date value in the @cdmFieldName field of the @cdmTableName table that occurs prior to birth.",Verification,Plausibility,Temporal,field_plausible_after_birth.sql,plausibleAfterBirth=='Yes' | ||
| FIELD,plausibleBeforeDeath,"The number and percent of records with a date value in the @cdmFieldName field of the @cdmTableName table that occurs after death.",Verification,Plausibility,Temporal,field_plausible_before_death.sql,plausibleBeforeDeath=='Yes' | ||
| FIELD,plausibleStartBeforeEnd,"The number and percent of records with a value in the @cdmFieldName field of the @cdmTableName that occurs after the date in the @plausibleStartBeforeEndFieldName.",Verification,Plausibility,Temporal,field_plausible_start_before_end.sql,plausibleStartBeforeEnd=='Yes' |
There was a problem hiding this comment.
I think you were using an older check descriptions file. Can you readd these new checks? (also for the v5.3 and v5.4 check description file)
| MEASUREMENT,MEASUREMENT_CONCEPT_ID,3019370,Meperidine [Mass/volume] in Urine by Confirmatory method,,,,,,,,,,,,,,,,,,,,,"8636,8713,8725,8748,8751,8817,8820,8837,8840,8842,8845,8859,8861,8950,9028,9503,9514,9530,9532,9560,9564,9625,32964,32965,44777535,44777592,44777638,45956701",5,Approved UNIT_CONCEPT_IDs for the given MEASUREMENT_CONCEPT_ID | ||
| MEASUREMENT,MEASUREMENT_CONCEPT_ID,3023548,Base deficit in Arterial blood,,,,,,,,,,,,,,,,,,,,,"8729,8736,8745,8749,8753,8839,8843,8875,9440,9490,9491,9501,9553,9557,9559,9575,9586,9587,9588,9591,9608,9621,9631,9632,9654,9673,45891014",5,Approved UNIT_CONCEPT_IDs for the given MEASUREMENT_CONCEPT_ID | ||
| MEASUREMENT,MEASUREMENT_CONCEPT_ID,3024653,FEV1,,,,,,,,,,,,,,,,,,,,,"8519,8583,8587,8686,9261,9263,9271,9277,9283,9285,9286,9287,9288,9292,9293,9296,9300,9301,9303,9304,9314,9316,9317,9318,9366,9367,9382,9383,9390,9391,9393,9394,9412,9416,9482,9486,9515,9520,9535,9606,9628,9643,9665,44777531,44777662",5,Approved UNIT_CONCEPT_IDs for the given MEASUREMENT_CONCEPT_ID | ||
| MEASUREMENT,MEASUREMENT_CONCEPT_ID,3025313,Albumin [Mass/volume] in Body fluid,,,,,,,,,,,,,,,,,,,,,"8636,8713,8725,8748,8751,8817,8820,8837,8840,8842,8845,8859,8861,8950,9028,9503,9514,9530,9532,9560,9564,9625,32964,32965,44777535,44777592,44777638,45956701",5,A |
There was a problem hiding this comment.
Is there a reason for not including 4041262 - Procedure on male genital system?
There was a problem hiding this comment.
The OMOP vocabulary hierarchy is wrong in some concepts here, HCPCS "procedures on prostate OR perineum" has both parents "procedures on prostate" and "procedures on perineum". but it's OR and these codes are used in females.
Once the vocabulary is fixed, we can add these
There was a problem hiding this comment.
I was not able to find the concepts in the example you are referring to.
In any case, I suggest to not wait for a vocabulary update, and already add a set of more specific concepts for male procedures that do not include prostate. e.g. these three:
- 4040708 - Procedure on scrotum
- 4043199 - Procedure on penis
- 4040577 - Procedure on testis
There was a problem hiding this comment.
yes, sorry, it was Procedure on scrotum that lead to concept:
2313992 | 93975 | Duplex scan of arterial inflow and venous outflow of abdominal, pelvic, scrotal contents and/or retroperitoneal organs; complete study
Procedure on prostate leads to the 2211770 76872 Ultrasound, transrectal, which can be done for females as well.
So, I ended up with the following list
4250917 - Operation on prostate
4077750 - Operation on scrotum
4043199 - Procedure on penis
4040577 - Procedure on testis
| /*violatedRowsBegin*/ | ||
| SELECT cdmTable.* | ||
| FROM @cdmDatabaseSchema.@cdmTableName cdmTable | ||
| INNER JOIN @cdmDatabaseSchema.person p |
There was a problem hiding this comment.
INNER is redundant. It is not used in the other queries (also in this file, on line 41 for join with cohort), so for consistency I would remove it (here and on lines 38 and 54)
There was a problem hiding this comment.
Also, indendation of the join is inconsistent with the denominator subquery. I would unindent this block one level and have joins on the same indent as the from. (I know, indendation is already not consistent across queries, and this bugs me more than it should)
There was a problem hiding this comment.
I'll fix that, but I took SQL from the plausibleGenderChecks, thought INNER JOIN is some kind of standard here, although it's redundant obviously, reminds me of 5 monkeys experiment :)
There was a problem hiding this comment.
lol! 🐒 i have seen the INNER floating around various other OHDSI SQL queries as well.
how about this, i will try to use copilot to lint/standardize all of the SQL queries. if it works, i'll add the change into my docs PR.
…se_descendants.sql, added missing check_description
| @@ -20,8 +20,9 @@ FIELD,plausibleValueHigh,The number and percent of records with a value in the @ | |||
| FIELD,plausibleTemporalAfter,The number and percent of records with a value in the @cdmFieldName field of the @cdmTableName that occurs prior to the date in the @plausibleTemporalAfterFieldName field of the @plausibleTemporalAfterTableName table.,Verification,Plausibility,Temporal,field_plausible_temporal_after.sql,plausibleTemporalAfter=='Yes' | |||
| FIELD,plausibleDuringLife,"If yes, the number and percent of records with a date value in the @cdmFieldName field of the @cdmTableName table that occurs after death.",Verification,Plausibility,Temporal,field_plausible_during_life.sql,plausibleDuringLife=='Yes' | |||
| FIELD,withinVisitDates,The number and percent of records not within one week on either side of the corresponding visit occurrence start and end date,Verification,Conformance,,field_within_visit_dates.sql,withinVisitDates=='Yes' | |||
| CONCEPT,plausibleGender,"For a CONCEPT_ID @conceptId (@conceptName), the number and percent of records associated with patients with an implausible gender (correct gender = @plausibleGender).",Validation,Plausibility,Atemporal,concept_plausible_gender.sql,plausibleGender!='' | |||
There was a problem hiding this comment.
could you actually please put the concept checks at the end of the file, after the new plausible temporal checks? just so all FIELD are grouped together, and then CONCEPT :)
This pull request contains measurement / unit checks cleaned (uses 180 measurements instead of 2000) and gender checks using descendant concepts.