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

update to version 0.2.3 #104

Merged
merged 47 commits into from
Sep 15, 2023
Merged

update to version 0.2.3 #104

merged 47 commits into from
Sep 15, 2023

Conversation

Tina815
Copy link
Contributor

@Tina815 Tina815 commented Sep 11, 2023

No description provided.

Moohan and others added 30 commits February 1, 2023 11:53
[`{scales}` 1.0.0](https://scales.r-lib.org/news/index.html#scales-100) is needed as that's when `number_bytes` was introduced.

I also renamed the internal function variables to make it easier to read.
These are now more specific than before.
Some of these tests had failed because when you don't supply a `ref_date` it will calculate the age today (using `Sys.Date()`), this meant that the 'people' are now older than when we first wrote the tests, so `age_from_chi` no longer returned the expected age.

I wrote a small helper function to work out what the 'expected age' should be now, given the date we first set the tests.
`capture_warnings` has been superseded by `expect_warning` so I've swapped to using that.
This is faster and more readable than `!`
I have also made a few other tweaks to the code to make it a bit faster, and have added some new tests to account for the new parameter.
…_chi_tests

Fix the tests covering `age_from_chi`
…n_year

A large speed up to `extract_fin_year`
…-quiet-parameter

#90 Add a `quiet` parameter to `format_postcode`
Tina815 and others added 11 commits September 4, 2023 13:26
Update installation instructions in the README
They will now throw an informative error, instead of the current behaviour where they will throw a warning but also return the expected result.

The tests have also been updated such that they now expect these functions to error.
These links were valid but were redirecting - they have now been replaced.
@Tina815 Tina815 requested a review from Moohan September 11, 2023 14:11
R/file_size.R Outdated Show resolved Hide resolved
Co-authored-by: James McMahon <james.mcmahon@phs.scot>
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #104 (4b3fcdb) into master (8848073) will decrease coverage by 1.95%.
The diff coverage is 100.00%.

❗ Current head 4b3fcdb differs from pull request most recent head fd51540. Consider uploading reports for the commit fd51540 to get more accurate results

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
- Coverage   99.25%   97.30%   -1.95%     
==========================================
  Files          13       13              
  Lines         400      408       +8     
==========================================
  Hits          397      397              
- Misses          3       11       +8     
Files Changed Coverage Δ
R/age_calculate.R 100.00% <ø> (ø)
R/dob_from_chi.R 100.00% <ø> (ø)
R/match_area.R 100.00% <ø> (ø)
R/sex_from_chi.R 100.00% <ø> (ø)
R/extract_fin_year.R 100.00% <100.00%> (ø)
R/file_size.R 100.00% <100.00%> (ø)
R/format_postcode.R 100.00% <100.00%> (ø)
R/rename.R 27.27% <100.00%> (-72.73%) ⬇️

@Moohan Moohan self-requested a review September 11, 2023 16:12
Copy link
Member

@Moohan Moohan left a comment

Choose a reason for hiding this comment

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

Looks good to me (and all checks passed)!

Since I wrote a lot of the code in this PR it would be good to have someone else have a look over it (maybe ask in the Teams channel?) but probably fine if not possible.

@Tina815 Tina815 merged commit 48dc06f into master Sep 15, 2023
14 checks passed
@Tina815 Tina815 deleted the development branch September 15, 2023 13:17
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.

2 participants