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

Hiv cs tx continuity #451

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Hiv cs tx continuity #451

wants to merge 8 commits into from

Conversation

DennisGibz
Copy link
Contributor

No description provided.

,Patient.Gender
,Patient.DOB
,[AgeGroup].DATIMAgeGroup
,FactARTHistory.AgeGroup
Copy link
Contributor

Choose a reason for hiding this comment

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

@DennisGibz not sure, I thought since this is a Fact we should be using an AgeGroupKey?

Copy link
Contributor

@nobert-mumo nobert-mumo left a comment

Choose a reason for hiding this comment

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

@DennisGibz not sure, I thought since this is a Fact we should be using an AgeGroupKey?

@DennisGibz
Copy link
Contributor Author

@nobert-mumo yes but DimAgegroup was bringing about discrepancy in numbers. We have changed the report to be a linelist.

@nobert-mumo
Copy link
Contributor

@nobert-mumo yes but DimAgegroup was bringing about discrepancy in numbers. We have changed the report to be a linelist.

@DennisGibz not sure I follow, do you mean we changed the fact to a report ?

Copy link
Contributor

@Marymary-dev Marymary-dev left a comment

Choose a reason for hiding this comment

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

Ensure the CohortYearMonth is set to EndofMonth for ease of filtering

@@ -23,7 +23,7 @@ BEGIN
,YEAR(TRY_CAST(DateConfirmedHIVPositiveKey AS DATETIME2)) As CohortYear
,TRY_CAST(DateConfirmedHIVPositiveKey AS DATETIME2) As CohortYearMonth
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the CohortYearMonth is set to EndofMonth for ease of filtering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Marymary-dev this has since been addressed.

@DennisGibz
Copy link
Contributor Author

@nobert-mumo yes but DimAgegroup was bringing about discrepancy in numbers. We have changed the report to be a linelist.

@DennisGibz not sure I follow, do you mean we changed the fact to a report ?

@nobert-mumo this was never a fact. We have factARTHistory as the source fact.

@nobert-mumo
Copy link
Contributor

@nobert-mumo yes but DimAgegroup was bringing about discrepancy in numbers. We have changed the report to be a linelist.

@DennisGibz not sure I follow, do you mean we changed the fact to a report ?

@nobert-mumo this was never a fact. We have factARTHistory as the source fact.

@DennisGibz I thought [NDWH].[dbo].[FactARTHistory] is a fact ? then we should be having an AgeGroupKey that we join to DimAgeGroup when building the report.

@DennisGibz
Copy link
Contributor Author

@nobert-mumo yes but DimAgegroup was bringing about discrepancy in numbers. We have changed the report to be a linelist.

@DennisGibz not sure I follow, do you mean we changed the fact to a report ?

@nobert-mumo this was never a fact. We have factARTHistory as the source fact.

@DennisGibz I thought [NDWH].[dbo].[FactARTHistory] is a fact ? then we should be having an AgeGroupKey that we join to DimAgeGroup when building the report.

Yeah. We can have the dim attached to the fact then consume the column downstream. I'll look at that.

@nobert-mumo
Copy link
Contributor

@DennisGibz , the table FactARTHistory seems to be a bit off from being a fact table? I think we need to design not to have the table denormalized but only contain the foreign dim keys

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