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

Merge March 24 update into Master #933

Merged
merged 68 commits into from
Mar 26, 2024
Merged

Merge March 24 update into Master #933

merged 68 commits into from
Mar 26, 2024

Conversation

Jennit07
Copy link
Collaborator

We have released the March 24 update on 20/03/24 so now we can clean up GitHub and merge the branch back into master

Jennit07 and others added 30 commits January 16, 2024 15:39
This was causing NSUs to show a social care id. This now resolves this.
@SwiftySalmon spotted an issue where NSUs were showing a social care id
attached. This was due to us matching on NSUs first and then SC Client
variables at the end. Previously we matched SC client variables first
and then NSUs onto the episode file. This difference occurred when we
split up matching on client variables at the year specific extracts.
This is a quick PR to reorder the functions to match our previous logic.
I have tested the 1920 file to check this and the issue is no longer
there.
Improved the function `create_demog_test_flags` function to include a
parameter for specifying `chi` or `anon_chi`. This is due to this
function being used for testing the extracts (which calls chi) and also
being used for testing the episode and individual file (which calls
anon_chi). This now allows flexibility for interchanging between the two
forms and simplifies the code by removing duplicated code.

Closes #850
SwiftySalmon and others added 25 commits February 13, 2024 15:30
Due to a coding error, the `dplyr::pick` was not working correctly when
trying to take the average for assigning the label `unassigned`. This
was causing NSUs to have the label `psychiatry` attached as this was the
next name available. The fix is to specify all of the columns needed to
take an average for assigning the label `unassigned`. This has a big
impact on the figures.

The  current 'live' files:

```
  service_use_cohort       n
   <chr>                <int>
 1 Elective Inpatient  488510
 2 Geriatric           293863
 3 Limited Daycases    725538
 4 Maternity           206488
 5 Multiple Emergency 1140659
 6 Outpatients        1102837
 7 Prescribing        3982957
 **8 Psychiatry         1601193**
 9 Routine Daycase     178739
10 Single Emergency   1116014
**11 Unassigned          250917**
12 Unscheduled Care   1718283
13 NA                   55697

```

after the fix: 

```
   service_use_cohort       n
   <chr>                <int>
 1 Elective Inpatient  488521
 2 Geriatric           293869
 3 Limited Daycases    725583
 4 Maternity           206488
 5 Multiple Emergency 1140688
 6 Outpatients        1102878
 7 Prescribing        3982958
 **8 Psychiatry          111859**
 9 Routine Daycase     178742
10 Single Emergency   1116042
**11 Unassigned         1740237**
12 Unscheduled Care   1718269
13 NA                   55180
```

Ive asked the source team if they can run through a test to see the
impact on their figures so i am opening this as a draft for now incase
further changes are needed.

closes #907
* Add missing sc variables for no sc data

* Fix code for including `_inc_dna` variables

* Remove commented line
…ning. (#913)

Fix bug - pop file paths breaking indiv file
Opening a draft PR for now. I think we need to review the care home
methodology before deciding if we need the quarter calculation
functions. I will highlight areas in the code which i am not sure about
as i want to make sure we pick up the correct logic.

For now i have concluded that we can remove the `check_qtr_format`
function so have removed this from the code.

Closes Issue #886
A small issue was identified when running targets. Linked with changes to the function `fix_sc_end_dates()`
* fix sc_client_lookup sc_send_lca

* fix an issue of get_pop_path

* Style code

* fix the rest of get_pop_path from get_datazone_pop_path

* Update documentation

* fix sc_send_lca

* add missing year column

* explicitly specify the argument year to avoid corruption of targets

* Update documentation

* new data pipeline with targets
remove create_individual_files from targets and append it to run_targets script

* minor changes

* Style code

* undo sc_send_lca bit

* Update targets scripts

* Remove top level targets scripts

---------

Co-authored-by: lizihao-anu <[email protected]>
Co-authored-by: Megan McNicol <[email protected]>
Co-authored-by: Jennit07 <[email protected]>
Co-authored-by: Jennifer Thom <[email protected]>
A small issue was identified when running targets. Linked with changes
to the function `fix_sc_end_dates()`
…own for processing times (#899)

* re-writing process_sc_all sds and alarm_telecare with data.table to improve the speed

* Update documentation

* Style code

* changes in line with new process_sc_all_sds dplyr version

* Style code

* remove duplicate columns

* remove duplicated columns

---------

Co-authored-by: lizihao-anu <[email protected]>
Co-authored-by: Megan McNicol <[email protected]>
* set a correct file permission

* update descriptions in process_tests function

* Update documentation

---------

Co-authored-by: lizihao-anu <[email protected]>
* Set up `get_sandpit_extract_path`

* Update documentation

* Update sc `all` data paths

* Write sandpit extract if file does not exist

* Style code

---------

Co-authored-by: Jennit07 <[email protected]>
Due to a bug in the file, the 22/23 file was filtering data for east
ayrshire only. This is due to an error with matching to the SG
completeness file. Improvements to the code here is necessary as this
was not picked up in the automated process and was a manual change when
this file updated. This is related to the issue #906

The episode and individual files for 22/23 need to be re-ran to correct
this.
@Jennit07 Jennit07 requested a review from rchlv March 26, 2024 12:23
Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (28)
anu
atrialfib
casewhen
chd
commiting
consulations
copd
cvd
Drugsand
eol
fcase
fifelse
gpooh
hbp
hefailure
hri
intzone
lcap
LCHO
lizihao
parkinsons
Physicaland
popluation
rdname
refailure
scotp
setorder
sourcedev
To accept these unrecognized words as correct, you could run the following commands

... in a clone of the [email protected]:Public-Health-Scotland/source-linkage-files.git repository
on the mar-23-update branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/Public-Health-Scotland/source-linkage-files/actions/runs/8435876857/attempts/1'

OR

To have the bot accept them for you, reply quoting the following line:
@check-spelling-bot apply updates.

Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionary

This includes both expected items (235) from .github/actions/spelling/expect.txt and unrecognized words (28)

Dictionary Entries Covers Uniquely
cspell:k8s/dict/k8s.txt 153 2 1
cspell:swift/src/swift.txt 53 2
cspell:npm/dict/npm.txt 302 2
cspell:php/dict/php.txt 1689 2
cspell:java/src/java.txt 2464 2

Consider adding them (in .github/workflows/spelling.yml) for uses: check-spelling/check-spelling@main in its with:

      with:
        extra_dictionaries:
          cspell:k8s/dict/k8s.txt
          cspell:swift/src/swift.txt
          cspell:npm/dict/npm.txt
          cspell:php/dict/php.txt
          cspell:java/src/java.txt

To stop checking additional dictionaries, add (in .github/workflows/spelling.yml) for uses: check-spelling/check-spelling@main in its with:

check_extra_dictionaries: ''
Pattern suggestions ✂️ (1)

You could add these patterns to .github/actions/spelling/patterns.txt:

# Automatically suggested patterns
# hit-count: 5 file-count: 2
# Compiler flags
(?:^|[\t ,"'`=(])-[DPWXYLlf](?=[A-Z]{2,}|[A-Z][a-z]|[a-z]{2,})

Errors (4)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors Count
ℹ️ candidate-pattern 2
❌ check-file-path 1
❌ ignored-expect-variant 13
ℹ️ no-newline-at-eof 1

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Copy link
Collaborator

@rchlv rchlv left a comment

Choose a reason for hiding this comment

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

Jen and I have reviewed the changes together, and we are happy to merge into master.

@Jennit07 Jennit07 merged commit a570e6a into master Mar 26, 2024
11 of 18 checks passed
@Jennit07 Jennit07 deleted the mar-23-update branch March 26, 2024 14:39
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