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

Viv3ckj/update viz #25

Merged
merged 6 commits into from
Oct 9, 2024
Merged

Viv3ckj/update viz #25

merged 6 commits into from
Oct 9, 2024

Conversation

viv3ckj
Copy link
Contributor

@viv3ckj viv3ckj commented Oct 7, 2024

Closes #22

Updated graphs with new colours, and correctly ordered values (only for region, ethnicity and age).
Would be good to see how the report with real data looks to get an idea on how to improve clarity, but should be okay for now.

@viv3ckj viv3ckj requested a review from milanwiedemann October 7, 2024 16:49
Copy link
Member

@milanwiedemann milanwiedemann left a comment

Choose a reason for hiding this comment

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

Great work, just some small suggestions

reports/pharmacy_first_report.Rmd Outdated Show resolved Hide resolved
reports/pharmacy_first_report.Rmd Show resolved Hide resolved
@@ -93,7 +121,7 @@ Links to the codelist for each analysis can be found beneath the relevant sectio

### Total population

```{r, message=FALSE, warning=FALSE, fig.height=4, fig.width=4}
```{r, message=FALSE, warning=FALSE}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep this? it makes sure all the panels are roughly the same size? or what was your reason for removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was an issue with the titles and axes being cropped out

@@ -220,7 +260,7 @@ Here we show the number of consultations for each of the Pharmacy First Clinical

### Total population

```{r, message=FALSE, warning=FALSE, fig.height=4, fig.width=4}
Copy link
Member

Choose a reason for hiding this comment

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

see comment above

analysis/measures_definition_pf_codes_conditions.py Outdated Show resolved Hide resolved
Comment on lines +71 to +94
df_measures$ethnicity <- factor(
df_measures$ethnicity,
levels = c("White", "Mixed", "Asian or Asian British",
"Black or Black British", "Chinese or Other Ethnic Groups",
"Missing"),
ordered = TRUE
)

df_measures$age_band <- factor(
df_measures$age_band,
levels = c("0-19", "20-39", "40-59",
"60-79", "80+",
"Missing"),
ordered = TRUE
)

df_measures$region <- factor(
df_measures$region,
levels = c("East", "East Midlands", "London",
"North East", "North West", "South East",
"South West", "West Midlands", "Yorkshire and The Humber",
"Missing"),
ordered = TRUE
)
Copy link
Member

Choose a reason for hiding this comment

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

this does what we want, but it would be could do this as one call using mutate. at some later point we should integrate this into the tidy_measures() function, for now I think it's ok if we do this outside of the function.

df_measures <- df_measures %>%
  mutate(
    ethnicity = factor(ethnicity, ...),
    region = factor(region, ...),
    age_band = factor(age_band, ...),
    sex = factor(sex, ...)
)

@milanwiedemann milanwiedemann linked an issue Oct 9, 2024 that may be closed by this pull request
Add ethnicity_from_sus codes for missing ethnicities
@viv3ckj viv3ckj merged commit 0733b26 into main Oct 9, 2024
1 check passed
@viv3ckj viv3ckj deleted the viv3ckj/update-viz branch October 9, 2024 10:35
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.

Add August 2024 data to the report Update report visuals
2 participants