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

Documentation for plausibleBeforeDeath and plausibleStartBeforeEnd #543

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

MaximMoinat
Copy link
Collaborator

Fixes #541. Needs to be rendered with pkgdown after review.

@MaximMoinat
Copy link
Collaborator Author

@katy-sadowski, some thoughts while writing the docs:
For plausibleStartBeforeEnd, I am doubting whether it is CDM Convention or Critical. I argue the latter, as end dates before start dates creates negative durations which might break analyses. This would also mean we should lower the threshold to 0%, always failing if any records affected.

For plausibleBeforeDeath, we have a 60 days grace period. I've explained it as that administrative events are expected until the next month. Or was this an arbitrary choice.

Copy link
Collaborator

@katy-sadowski katy-sadowski left a comment

Choose a reason for hiding this comment

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

Woohooo thanks so much Maxim!

Re: the 60 day buffer - I found the explanation on the Themis docs: https://ohdsi.github.io/Themis/obs_periods_for_ehr.html#observation-period-end-date. I do wonder if this warrants its own general convention for "How to handle events dating after death"? @clairblacketer what do you think? [PS - super exciting to be able to search for and find that on the website 😄 ]

Re: start before end - interesting question. I am torn, because this could cause really serious problems in some cases, maybe even errors in code, so that would be an argument for Fatal. On the other hand I think the intention of Fatal was really database conformance level stuff. @clairblacketer what do you think?

vignettes/checks/plausibleStartBeforeEnd.Rmd Outdated Show resolved Hide resolved
vignettes/checks/plausibleStartBeforeEnd.Rmd Outdated Show resolved Hide resolved
@MelaniePhilofsky
Copy link

We do have a ratified convention for events after death. It needs to be documented in the Themis convention library. Right now it is just a ratified and closed in the Themis issues OHDSI/Themis#42
I will reopen it as a "needs documentation"

Copy link
Collaborator

@katy-sadowski katy-sadowski left a comment

Choose a reason for hiding this comment

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

Thank you @MaximMoinat !

Maybe we can discuss Fatal vs Convention for StartBeforeEnd at the next DQ call. Can merge in for now and update if we decide to change it later :)

@katy-sadowski katy-sadowski merged commit 0c8e4d8 into develop Jun 12, 2024
2 of 8 checks passed
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.

3 participants