Skip to content

Inequalities info #189

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Inequalities info #189

wants to merge 15 commits into from

Conversation

DCEW
Copy link
Contributor

@DCEW DCEW commented Mar 12, 2025

Added a chapter on the inequalities methodology with reproduced plots based on @yiwen-h 's artwork (and using dummy data). No idea of the level of detail really needed here, so consider this a first draft.

@ChrisBeeley
Copy link
Member

Not to be pedantic but should this PR be marked as draft?

@DCEW
Copy link
Contributor Author

DCEW commented Mar 12, 2025

Not sure tbh - I am submitting it to you both for review now, but my mention of 'draft' was more to suggest I'm expecting plenty of changes will need made before it'd done.

@yiwen-h
Copy link
Member

yiwen-h commented Mar 12, 2025

Think we need to add patchwork to renv - it's stopping the preview deployment

@DCEW
Copy link
Contributor Author

DCEW commented Mar 12, 2025

Ah ok I'll snapshot and push

@yiwen-h
Copy link
Member

yiwen-h commented Mar 12, 2025

Thank you! FYI @ChrisBeeley the preview deployment lives here: https://connect.strategyunitwm.nhs.uk/nhp_project_information_preview/

@ChrisBeeley
Copy link
Member

I was literally just looking at this and thinking I didn't know where the preview was and would need to pull it down and render locally- thank you 😃

ChrisBeeley
ChrisBeeley approved these changes Mar 13, 2025
View reviewed changes
Copy link
Member

@ChrisBeeley ChrisBeeley left a comment

Choose a reason for hiding this comment

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

Looks great, thank you. I've made a couple of comments on the language, which are mostly just me being a cranky old man 🧓 , use or discard as you deem fit

DCEW and others added 5 commits March 13, 2025 09:04
@DCEW @ChrisBeeley
Update modelling_methodology/inequalities.qmd
0f04370 
Add NHSE health inequalities link

Co-authored-by: Chris Beeley <[email protected]>
@DCEW @ChrisBeeley
Update modelling_methodology/inequalities.qmd
ec2d1f3 
Remove the word 'grossly'

Co-authored-by: Chris Beeley <[email protected]>
@DCEW
Adjust wording in line with CB review and remove Figure_1.png
b804ace
@DCEW
Text alterations after review
cb6c4f3 
Merge branch 'inequalities_info' of https://github.com/The-Strategy-Unit/nhp_project_information into inequalities_info

# Conflicts:
#	modelling_methodology/inequalities.qmd
@DCEW
Add missing health inequalities link
0cc462c
yiwen-h
yiwen-h requested changes Mar 13, 2025
View reviewed changes
Copy link
Member

@yiwen-h yiwen-h left a comment

Choose a reason for hiding this comment

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

Two more things:

  1. We should link to @swyatt-SU published report on this which the whole thing is based on

  2. I think that as well as the diagrams we should provide a table with the numbers in as well. Because the model is so complicated, I've found worked examples with actual numbers can be helpful for understanding what's actually happening

@DCEW
Copy link
Contributor Author

DCEW commented Mar 13, 2025

Urg merge conflict. Who usually merges - the codeowner or PR submitter? I've added the gt and patchwork packages and snapshotted, FYI.

@yiwen-h
Copy link
Member

yiwen-h commented Mar 13, 2025

In this team it's the PR submitter who usually sorts merge conflicts and does the actual merges :)

https://the-strategy-unit.github.io/data_science/style/git_and_github.html#reviewing-a-pr

@DCEW
Copy link
Contributor Author

DCEW commented Mar 14, 2025

Resolved merge conflict

@DCEW DCEW requested a review from yiwen-h March 14, 2025 10:11
@yiwen-h
Copy link
Member

yiwen-h commented Mar 14, 2025

ugh something is up with the renv.lock file?

@DCEW
Copy link
Contributor Author

DCEW commented Mar 14, 2025

Fixed I hope...

yiwen-h
yiwen-h requested changes Mar 14, 2025
View reviewed changes
@yiwen-h
Copy link
Member

yiwen-h commented Mar 14, 2025

Once more amendments made, think it would also be good to get a review from someone who is further away from this for an external perspective. Maybe @ghobro or @jacgrout ?

@ghobro
Copy link

ghobro commented Mar 14, 2025

Once more amendments made, think it would also be good to get a review from someone who is further away from this for an external perspective. Maybe @ghobro or @jacgrout ?

I should be able to do that as have been spending a lot of time on IMD recently for another project. When would you want the review by?

@DCEW
Copy link
Contributor Author

DCEW commented Mar 14, 2025 •
edited
Loading

Once more amendments made, think it would also be good to get a review from someone who is further away from this for an external perspective. Maybe @ghobro or @jacgrout ?

I should be able to do that as have been spending a lot of time on IMD recently for another project. When would you want the review by?

ASAP really Gabriel, as its part of the current sprint. I'll try to get the changes done now. Thanks for offering!

@ghobro
Copy link

ghobro commented Mar 14, 2025

Once more amendments made, think it would also be good to get a review from someone who is further away from this for an external perspective. Maybe @ghobro or @jacgrout ?

I should be able to do that as have been spending a lot of time on IMD recently for another project. When would you want the review by?

ASAP really Gabriel, as its part of the current sprint. I'll try to get the changes done now. Thanks for offering!

OK I will do look through this afternoon.

@yiwen-h
Copy link
Member

yiwen-h commented Mar 14, 2025

it's ok, no need to rush - we have till 24th March @ghobro @DCEW

@DCEW DCEW force-pushed the inequalities_info branch from 4c7c130 to 1969f87 Compare March 17, 2025 09:31
@yiwen-h
Copy link
Member

yiwen-h commented Mar 17, 2025

If you have resolved the issues I raised above, please click the "Resolve conversation" button for each issue and then re-request review.

yiwen-h
yiwen-h requested changes Mar 17, 2025
View reviewed changes
@DCEW DCEW requested review from yiwen-h and ghobro March 17, 2025 11:12
@yiwen-h yiwen-h requested a review from ChrisBeeley March 17, 2025 11:21
ghobro
ghobro reviewed Mar 17, 2025
View reviewed changes
@DCEW DCEW requested a review from ghobro March 18, 2025 09:41
yiwen-h
yiwen-h approved these changes Mar 19, 2025
View reviewed changes
Copy link
Member

@yiwen-h yiwen-h left a comment

Choose a reason for hiding this comment

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

We got there in the end! Thanks for getting this together. However we will not be merging into main until code is in the model (hopefully in v3.4)

ChrisBeeley
ChrisBeeley approved these changes Mar 24, 2025
View reviewed changes
Copy link
Member

@ChrisBeeley ChrisBeeley left a comment

Choose a reason for hiding this comment

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

Tiny, tiny comment, but all good

@DCEW DCEW self-assigned this Apr 11, 2025
@DCEW DCEW mentioned this pull request May 2, 2025
Closed
@Josh-Cook3 Josh-Cook3 mentioned this pull request May 19, 2025
Merged
@matt-dray matt-dray mentioned this pull request Jun 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisBeeley ChrisBeeley ChrisBeeley approved these changes

@ghobro ghobro ghobro approved these changes

@yiwen-h yiwen-h yiwen-h approved these changes

Assignees

@DCEW DCEW

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DCEW @ChrisBeeley @yiwen-h @ghobro