Skip to content

✨ Split out INCLUDE specific fields#45

Closed
RobertJCarroll wants to merge 1 commit into
mainfrom
include_participant
Closed

✨ Split out INCLUDE specific fields#45
RobertJCarroll wants to merge 1 commit into
mainfrom
include_participant

Conversation

@RobertJCarroll
Copy link
Copy Markdown
Collaborator

@RobertJCarroll RobertJCarroll commented Apr 24, 2026

This moves some INCLUDE specific fields into a different class.

It demonstrates:

  1. The inheritance approach- because IncludeParticipant is_a demographics it has all of those fields in the generated model files: include_access_model.sql . We might want to approach this a different way, eg, just with separate tables that link.
  2. Splitting out Program-specific fields within the same repo. We might want to have other repositories with those model additions.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://include-dcc.github.io/include-access-model/pr-preview/pr-45/

Built to branch gh-pages at 2026-05-05 20:34 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@Christina-J-Diaz
Copy link
Copy Markdown
Contributor

I think it makes sense to have down syndrome status here. Age at engagement - this one I feel we sometimes do receive for KF studies but we call it "age at enrollment" . Not sure if that one should be "include only".

Then lastly - what does the inheritance look like in terms of translating this to dbt models? Not sure if I'm understanding that piece clearly.

@RobertJCarroll
Copy link
Copy Markdown
Collaborator Author

I'm also considering mixins for the program-specific fields. We'd have a class with just the additional fields, then a class that is_a core table and has the mixin of the program-specific class. That would give the separate tables and one merged. It does require the model to be more complicated, though, and we might not want to use that additive flow.

This moves some INCLUDE specific fields into a different class.
@RobertJCarroll RobertJCarroll force-pushed the include_participant branch from 202ace5 to 1dbbaa0 Compare May 5, 2026 20:33
@RobertJCarroll
Copy link
Copy Markdown
Collaborator Author

I think it makes sense to have down syndrome status here. Age at engagement - this one I feel we sometimes do receive for KF studies but we call it "age at enrollment" . Not sure if that one should be "include only".

We have it a little broader than just the "age at enrollment" as we don't always have that information. Sometimes they are the same age. We could keep it in the core model if it's something that KF may use, too.

Then lastly - what does the inheritance look like in terms of translating this to dbt models? Not sure if I'm understanding that piece clearly.

It means the shared tables exists and then there are additional tables with all of the shared table fields and the extension fields. I just realized (and fixed) the PR previews not fully working, so I hope that helps make it clearer. The demographics table has its fields, all of which are included in the INCLUDE Participant table as well. If you look at the bottom of the INCLUDE participant page, you can see the short definition and the full definition (which includes every slot from Demographics):
image

@RobertJCarroll
Copy link
Copy Markdown
Collaborator Author

The more I've been thinking about it, the more I think we should separate out the repos. It'd be a big change in repo structure potentially, but I think it might support wider use (eg, AnVIL wouldn't add a third set of bonus tables or someone else entirely could pick up the core model for NCPI FHIR) and more ability to configure targets (eg, I could build the models with no extras, KF, INCLUDE, or both).

@RobertJCarroll
Copy link
Copy Markdown
Collaborator Author

I am closing this out and moving the changes to the new repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants