search

OHDSI / CohortDiagnostics

An R package for performing various cohort diagnostics.
https://ohdsi.github.io/CohortDiagnostics
42 stars 49 forks source link

Vocab table schema for exportConceptInformation #1146

Open lrasmus opened 2 weeks ago

lrasmus commented 2 weeks ago

When running executeDiagnostics, I specify cdmDatabaseSchema and vocabularyDatabaseSchema. I received an error that the concept table could not be found - but the error message was referencing the schema defined in cdmDatabaseSchema, not vocabularyDatabaseSchema.

I noticed that in exportConceptInformation, only the cdmDatabaseSchema is being passed in. And the function seems to purposefully reference the cdmDatabaseSchema when looking for the concept (and other vocabulary related) tables.

I had expected it to use vocabularyDatabaseSchema throughout for vocabulary-related tables. I wasn't sure if there was a design decision here on why cdmDatabaseSchema should instead be used. If it is reasonable to use vocabularyDatabaseSchema in this function, I would be happy to propose the change and submit a PR, but wanted to confirm first that we'd want this changed.

azimov commented 1 week ago

@lrasmus This does sound like a bug. In general, most users implement the pattern that the CDM contains all the vocabulary tables directly rather than having a separate schema (I would consider this a best practice as vocabularies change frequently and you will almost certainly want you ETL to be tied to a vocabulary release). However, allowing a separate vocabulary schema should be supported so a PR allowing the use of a separate vocabulary schema would be accepted. As you say, adding the vocabulary schema variable here should resolve this issue.