get_spd(): get spd archived files #19
Conversation
bnowok
commented
May 7, 2025
bnowok
commented
- Reading in older versions of SPD file regardless of file format.
- Some additional error messages.
- Small changes to help file.
# Conflicts: # .github/workflows/document.yaml # .github/workflows/pkgdown.yaml # .github/workflows/style.yaml # R/data.R # R/read_file.R # data-raw/hscp_locality_variables.R # data-raw/simd_datazone_variables.R # data-raw/simd_postcode_variables.R # data-raw/spd_variables.R # data/hscp_locality_variables.rda # data/simd_datazone_variables.rda # data/simd_postcode_variables.rda # data/spd_variables.rda # man/find_latest_file.Rd # man/hscp_locality_variables.Rd # man/read_file.Rd # man/simd_datazone_variables.Rd # man/simd_postcode_variables.Rd # man/spd_variables.Rd
Moohan
left a comment
Moohan
left a comment
There was a problem hiding this comment.
Looks good, and seems to work for me. Some minor suggestions + a suggestion to refactor out a function.
It would also be great to add tests for reading various archive files, at least:
- An archive file that works
- An archive date that is valid but doesn't exist
- An invalid archive date
Co-authored-by: James McMahon <james.mcmahon@phs.scot>
Co-authored-by: James McMahon <james.mcmahon@phs.scot>
|
It's working well for me! It would be good to give users a way to check the available archive versions that exist in the archive folder. While there is an example in the documentation indicating that '2023_2' exists, I as a user am not aware of any of the other versions without delving into the archive folder, of which the location is not advertised anywhere. In the future we could include helper functions that return file paths, including file paths for archived files to allow for user meta-programming where they might for example want to check for the existence of files and perform actions based on that. |
|
Just off the back of @Nic-Chr's comment: It might be nice to include the code for the current version in the message: |
Co-authored-by: Nick Christofides <74770457+Nic-Chr@users.noreply.github.com>
Moohan
left a comment
Moohan
left a comment
There was a problem hiding this comment.
Happy that this all works as expected. Tests also cover enough cases for now!
3 participants
