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

Payer plan period criteria #477

Closed
wants to merge 2 commits into from

Conversation

raomane
Copy link

@raomane raomane commented Oct 4, 2017

@anthonysena
Copy link
Collaborator

Hi @raomane - thanks for contributing this work and apologies for the delay in providing a review. Part of that delay stems from the fact that this PR introduces new functionality that is designed to work against CDM v5.3 which wasn't officially released until early 2018.

We discussed this work on the last OHDSI Architecture call and had a good discussion around how we want to introduce new functionality like this into the platform that targets newer features of the CDM. While there is still more discussion required on how to best handle CDM version changes in the tools long term, we did decide on a short term approach. Our short-term approach will be to add a warning in the Payer Plan Period criteria template informing users that when using this criteria, it will only work against a CDM that is using v5.3 features.

We'd like to get this work into the upcoming release and do that, we'd kindly ask that you rebase your work off the latest work in the Atlas master branch. Once that is done, can you re-establish this PR? @fdefalco has volunteered to add the warning mentioned above so that we can have this put into the upcoming build.

I'm also going to link this PR with a discussion that @chrisknoll started on the Circe repo as it is directly related: OHDSI/circe-be#27.

Tagging @gowthamrao for awareness as well.

Please reach out if you have any questions or concerns.

Copy link
Collaborator

@anthonysena anthonysena left a comment

Choose a reason for hiding this comment

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

Per my other comment, can you rebase your work off the latest work in Atlas/master?

@gowthamrao
Copy link
Member

tagging @pavgra and Greg

@chrisknoll
Copy link
Collaborator

Hi, I can manage adding the 2 features: payer plan period criteria and censor windows into 1 PR. I'll submit a new PR once it's ready (give me a few hours) and I'll close these others out once they become redundant.

@chrisknoll
Copy link
Collaborator

Covered in #569

@chrisknoll chrisknoll closed this Feb 13, 2018
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.

4 participants