-
Notifications
You must be signed in to change notification settings - Fork 456
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
Visit_detail_concept_id #145
Comments
I vote we keep it as visit_detail_concept_id. It can contain the same values as |
Thank you. I am fine with both, but then we will need to change visit_source_concept_id to visit_detail_source_concept_id. How about visit_detail_type_concept_id etc |
In response to the conversation today on the WG call: It seems to me there is a common convention in the CDM to avoid using the duplicate field names in different tables unless the field is a foreign key to another clinical data table. For instance, for any reference to person the foreign key is 'person_id', for any reference to care site the foreign key is 'care_site_id', etc, no matter the table. If, however, the field isn't a key to another table (e.g. x_date) or references a vocabulary table (e.g. x_concept_id) then the field name is unique across all tables. observation_concept_id vs. procedure_concept_id, observation_date vs procedure date, etc. I believe the benefit of keeping this convention is two-fold: Anyway, my suggestion is to avoid using any of the same field names as the visit_occurrence table unless its a foreign key to the visit_occurrence table itself. visit_detail_concept_id or detail_concept_id or finding another name for a visit_detail, which to me is more of a short visit (encounter?) than a fact about a visit. |
Thank you @rtmill and @clairblacketer - i agree with changing the names (makes sense), and everything is < 30.
|
Do we think vdetail_concept_id would be allowed? To disambiguate from other future detail tables.
Or visitd_concept_id .
George
From: Robert Miller [mailto:notifications@github.com]
Sent: Tuesday, January 09, 2018 2:32 PM
To: OHDSI/CommonDataModel <CommonDataModel@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [OHDSI/CommonDataModel] Visit_detail_concept_id (#145)
In response to the conversation today on the WG call:
It seems to me there is a common convention in the CDM to avoid using the duplicate field names in different tables unless the field is a foreign key to another clinical data table. For instance, for any reference to person the foreign key is 'person_id', for any reference to care site the foreign key is 'care_site_id', etc, no matter the table.
If, however, the field isn't a key to another table (e.g. x_date) or references a vocabulary table (e.g. x_concept_id) then the field name is unique across all tables. observation_concept_id vs. procedure_concept_id, observation_date vs procedure date, etc.
I believe the benefit of keeping this convention is two-fold:
-Clarity on definitions of concepts and mappings (can use concept class/domain to specify field)
-Avoidance of errors caused by ambiguity of variable names of joins
Anyway, my suggestion is to avoid using any of the same field names as the visit_occurrence table unless its a foreign key to the visit_occurrence table itself. visit_detail_concept_id or detail_concept_id or finding another name for a visit_detail, which to me is more of a short visit (encounter?) than a fact about a visit.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#145 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AGRVXviEavZW3it1J-kJ6TfqKtwlwq77ks5tI76tgaJpZM4RVT9v>.
|
So I've been doing some thinking... (this is never good) What if instead we get rid of the visit_detail table altogether and just add an optional parent_visit_id field to the visit_occurrence table? That's the only difference in the tables right? If visit_detail is capable of being a parent visit to another visit_detail record (relations can be hierarchical or sequential according to wiki), then really the two tables are functionally equivalent. If you are only looking for what we are currently defining as visit_occurrences you could just take the visits that have parent_visit_id as NULL. Reasoning: I would guess that I'm missing something? |
@rtmill yes, we went thru this in the CDM WG. Should we have a long table that self-references itself. Lot of reasons were expressed to not to want to do this; including confusion, technical challenges etc |
Also as statued there #153
|
closing - this was addressed in v5.3.2 though #153 is still open |
CommonDataModel/Sql Server/OMOP CDM sql server ddl.txt
Line 326 in ced0af0
Are we using visit_detail_concept_id or visit_concept_id for visit_detail table? There is a clash between documentation and DDL for this field https://github.com/OHDSI/CommonDataModel/blob/master/Documentation/CommonDataModel_Wiki_Files/StandardizedClinicalDataTables/VISIT_DETAIL.md
Documentation says it is visit_concept_id, although visit_detail_concept_id is less ambiguous, but visit_concept_id was proposed to the CDM workgroup as part of the visit_detail proposal.
I like the visit_detail_concept_id, to avoid ambiguity - but the concept_id is still belongs to the 'Visit' domain (not 'Visit detail' domain). If we decide to go with visit_detail_concept_id, then we will also need to use visit_detail_source_concept_id. Current implementation is visit_source_concept_id
CommonDataModel/Sql Server/OMOP CDM sql server ddl.txt
Line 338 in ced0af0
The text was updated successfully, but these errors were encountered: