-
Notifications
You must be signed in to change notification settings - Fork 1
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
Bring main up to date for the January 2025 release #147
Draft
Moohan
wants to merge
310
commits into
main
Choose a base branch
from
development
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+2,909
−2,712
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have commented these out for now but probably it would make sense to include `lp_path` in `Global Script.R`, and then it would definitely only be needed in one place.
I quickly looked into #83 and noticed that the logic for `max_fy` in `Unscheduled Care/2. Unscheduled Care outputs.R` wasn't correct - It would never be evaluated as TRUE... I've tweaked it to what I think it was supposed to be doing i.e. use the current FY if in Q4 and use the previous FY otherwise. I also removed the custom `fy()` function and replaced it with `phsmethods::extract_fin_year()` which returns the same value but is much faster (3500x 😆). The `fy()` function didn't have any values for 2024/25 so I suspect was returning NA, hence the odd output in the report we saw. I think #83 should still be open as it would be good to tie `max_fy` to the populations rather than the current date.
The general aim here is to make the code a bit cleaner. The main change is to `age_group_1` which was pretty horrible to decipher 😆
I noticed there were one or two of these dotted about. I don't think it will make any difference since we're outputting to Word anyway but I think it's cleaner to use standard quotes instead of the ones Word will use ('smart quotes'). Normal `'` Smart quotes `‘’`
Without this addition, the new code would fail on any run after the first as the `loop_env' object would be removed at the end of the first loop.
This is a repeat of #78 but with the issue fixed! The previous one worked for the first iteration of the loop, but then it was deleting the `loop_env` object so at the end of a subsequent iteration it would not be able to 'work out' which objects to keep and which should be removed ```r rm(list = setdiff(ls(), loop_env)) ``` This is the line at the end of the loop which essentially finds the list of objects in the environment which weren't in the environment before the loop started (`setdiff`) and then removes them (`rm(list = x)` where `x` is a character vector with names of objects. I tested this on a 3 Locality HSCP (Orkney Isles) and it didn't throw any errors and the output also seems fine. This was with the default 4096 MB RAM!
Co-authored-by: James McMahon <[email protected]>
This is a minor QOL change that's probably only relevant in `1. Unscheduled Care data extraction.R`. It uses `Sys.getenv("USER")` to get the username from the system environment, so only the password is required to be entered. I just copy/pasted to make sure it's the same code in all locations to avoid any confusion as to why one is different from another.
…m revised Scotpho psych hosp adm data
…Health-Scotland/list-localities-profiles into services_rm_scotpho_stat
- Data folder created and relevant scotpho data downloaded into it - Relevant code updated to point to new data folder - testing markdown checked against master markdown - changes made to ensure the same- mostly adding in fig cap differences - Master markdown checked and minor changes made such as adding in 'the' in few places Testing markdown available to view and check here: \\nssstats01\LIST_analytics\West Hub\02 - Scaled Up Work\RMarkdown\Locality Profiles\Lifestyle & Risk Factors
Update year and update column based upon new Scotpho data (downloaded and in new 2024 folder).
Have ctrl + F the whole code and access dep only appears to show up in files changed in this PR
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@khansh01 - This PR might be a useful resource for listing the changes we have made.