-
Notifications
You must be signed in to change notification settings - Fork 457
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
Clarify the definition of "required" #554
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never realised that this might be confusing, but I can see that this sentence helps. Thanks Adam!
Could you also generate the html (render_site()
) so it will be displayed on the github.io page. (to my knowledge this not triggered automatically upon merge).
Sure I can do that. Just wanted to make it clear that the tables and fields need to exist even if they're not required. |
@ablack3 I am going through to clear some technical debt and am currently reviewing PRs. I don't agree with the statement that 'required' fields do not need to be populated. They all should be populated with non-null values, but any data that is not available, like race_concept_id, can be mapped to zero. |
@clairblacketer The sentence that Adam has added "Fields that are not required should exist in the CDM table but do not need to be populated." I think we can agree with that. |
Ah thanks Maxim! I misread. I agree with the statement and I’ll pull in the pr |
I also recently learned that CDM columns can be in any order. Maybe that's clear to folks coming from a database background. I guess databases don't have a strict notion of column order. |
@ablack3 @MaximMoinat I am closing this PR because of some conflicts in the html files but I took the text and have since updated the website. |
CDM fields are marked as required or not but it isn't obvious what exactly it means for a field to be required. This PR adds a sentence that makes is clear that non-required fields must still be present in the database but do not need to be populated with data.
#535
OHDSI/OpenSourceCommunity#8