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

Bugfix/numerator obs values #260

Merged
merged 10 commits into from
Feb 15, 2024
Merged

Conversation

RohitKandimalla
Copy link
Contributor

@RohitKandimalla RohitKandimalla commented Feb 9, 2024

Pull requests into cqm-execution require the following. Submitter and reviewer should ✅ when done. For items that are not-applicable, note it's not-applicable ("N/A") and ✅.

Case: While generating observation results for each episode, we verify if the episode is a part of Numerator or Denominator, When Episode is not a part of Denominator, then the first index of observation_values array for that episode should be defaulted to 0.
Bug: Instead of adding 0 at the first index, we are replacing the existing value, which could be the value of Numerator Observation.

https://jira.cms.gov/browse/MAT-6623
Screenshot of the contents from This story
image

Submitter:

  • This pull request describes why these changes were made.
  • Internal ticket for this PR:
  • Internal ticket links to this PR
  • Code diff has been done and been reviewed
  • Tests are included and test edge cases
  • Tests have been run locally and pass

Reviewer 1:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

Reviewer 2:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

… are cases when the observation values could alredy have 2 values in the array
@RohitKandimalla RohitKandimalla removed the request for review from dczulada February 9, 2024 17:57
@dczulada
Copy link
Contributor

@RohitKandimalla is it possible to post the contents of the JIRA issue (https://jira.cms.gov/browse/MAT-6623 ) in this pull request? I'm having trouble understanding what this is trying to fix.

@RohitKandimalla
Copy link
Contributor Author

@RohitKandimalla is it possible to post the contents of the JIRA issue (https://jira.cms.gov/browse/MAT-6623 ) in this pull request? I'm having trouble understanding what this is trying to fix.

Sure I add a screenshot, so that it will easier to understand c&p the contents. The over all issue is that Numerator Observations results are not returned by cqm-execution.

Technically, cql-execution is returning us the num_obs values, but during the construction of observation_values array, we are overriding existing values instead of unshifting the array.

adongare
adongare previously approved these changes Feb 14, 2024
dczulada
dczulada previously approved these changes Feb 15, 2024
@RohitKandimalla RohitKandimalla dismissed stale reviews from dczulada and adongare via b5a8f87 February 15, 2024 15:27
@RohitKandimalla RohitKandimalla merged commit e1d9e34 into master Feb 15, 2024
6 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.

4 participants