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

OMOP CDM docs #59

Merged
merged 14 commits into from
May 16, 2023
Merged

OMOP CDM docs #59

merged 14 commits into from
May 16, 2023

Conversation

Iain-S
Copy link
Collaborator

@Iain-S Iain-S commented Mar 20, 2023

Work in progress.

@Iain-S Iain-S added the WIP Work in Progress (do not merge) label Mar 20, 2023
@Iain-S Iain-S force-pushed the iain-s/omop-docs branch from 332014f to bc2f703 Compare March 20, 2023 12:53
@Iain-S Iain-S force-pushed the iain-s/omop-docs branch from bc2f703 to 0ac860f Compare April 17, 2023 15:23
@Iain-S Iain-S force-pushed the iain-s/omop-docs branch from 6e9a6d4 to 170dd54 Compare April 25, 2023 14:14
@Iain-S Iain-S force-pushed the iain-s/omop-docs branch from 8d43f07 to 76e8e72 Compare April 28, 2023 10:19
@Iain-S Iain-S force-pushed the iain-s/omop-docs branch from 76e8e72 to efe11b0 Compare May 5, 2023 17:05
@Iain-S
Copy link
Collaborator Author

Iain-S commented May 5, 2023

Today, @cptanalatriste and I made changes / additions until we were able to successfully run make-tables, make-generators, create-tables, create-generators and create-data on the cms_desynpuf_sample_53_large schema:

  • could be in a separate PR truncated String col types to account for a couple of VARCHAR(1)s
  • made a custom config and generators file to account for the combined pk & fk of death.person_id
  • could be in a separate PR returned None (IE no rows) from ColumnValueProvider rather than erroring if there are no rows in the table using getattr(...,None)

Still to do:

  • add tests for the two code changes
  • optionally split the two code changes into a separate PR as they aren't OMOP-specific
  • rebase on main / merge in changes from main
  • in the example omop config, we commented out the cohort def and attribute def tables so that they aren't treated as vocab. we should either explain this in the docs or add some data to those tables and reinstate them as vocabs (I'd prefer the latter)

@cptanalatriste
Copy link
Collaborator

cptanalatriste commented May 9, 2023

@Iain-S I will create new issues for the suggested PRs and work on them.

@Iain-S Iain-S requested a review from cptanalatriste May 16, 2023 15:38
Copy link
Collaborator

@cptanalatriste cptanalatriste left a comment

Choose a reason for hiding this comment

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

Looks good to me. Well done!

@cptanalatriste cptanalatriste merged commit d098b1e into main May 16, 2023
@cptanalatriste cptanalatriste deleted the iain-s/omop-docs branch May 16, 2023 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in Progress (do not merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants