Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ Imports:
lifecycle,
pipfun (>= 0.3.10),
glue,
qs,
pins,
stamp
stamp,
qs2
Suggests:
withr,
rmarkdown,
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export(as_pipid)
export(as_pipmd)
export(assign_pipclass)
export(aux_ids)
export(check_dlw_id_name)
export(find_dlw_data)
export(find_pip_data)
export(get_from_piploadenv)
Expand Down
17 changes: 5 additions & 12 deletions R/OLD_pip_load_aux.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#' @export
#'
#' @examples
#' \dontrun{
#' # Load CPI
#' cpi <- pip_load_aux("cpi")
#'
Expand All @@ -49,7 +50,6 @@
#' head(av)
#' df <- pip_load_aux(measure, version = av[1])
#' head(df)
#' \dontrun{
#' df <- pip_load_aux(measure, version = -1)
#' head(df)
#' df <- pip_load_aux(measure, version = "pick")
Expand Down Expand Up @@ -332,20 +332,13 @@ read_by_format <- function(file_to_load) {
pformat <- fs::path_ext(file_to_load)

x <-
if (pformat == "qs") {

qs::qread(file_to_load)

} else if (pformat == "fst") {

if (pformat == "qs2") {
qs2::qs_read(file_to_load)
} else if (pformat == "fst") {
fst::read_fst(file_to_load, as.data.table = TRUE)

} else if (pformat == "rds") {

readr::read_rds(file_to_load)

} else if (pformat == "dta") {

haven::read_dta(file_to_load)
}
Comment on lines 334 to 343
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Unsupported extensions are not rejected explicitly in read_by_format().

If pformat is unknown, x is left NULL and returned silently, which can fail later in less obvious ways. Fail fast with a clear abort at the read boundary.

🔧 Proposed fix
 read_by_format <- function(file_to_load) {
 
   pformat <- fs::path_ext(file_to_load)
 
   x <-
     if (pformat == "qs2") {
       qs2::qs_read(file_to_load)
     } else if (pformat == "fst") {
       fst::read_fst(file_to_load, as.data.table = TRUE)
     } else if (pformat == "rds") {
       readr::read_rds(file_to_load)
     } else if (pformat == "dta") {
       haven::read_dta(file_to_load)
+    } else {
+      cli::cli_abort(c(
+        "Unsupported file format",
+        x = "Extension {.field {pformat}} is not supported.",
+        i = "Supported formats: {.field {c('qs2', 'fst', 'rds', 'dta')}}"
+      ), class = "pipload_error")
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@R/OLD_pip_load_aux.R` around lines 334 - 343, The read_by_format branch
leaves x NULL when pformat is unrecognized; update read_by_format (the branch
that sets x based on pformat) to explicitly error when pformat is not one of the
supported values ("qs2", "fst", "rds", "dta") by calling abort/stop with a clear
message that includes the invalid pformat and the allowed formats so the
function fails fast instead of returning NULL.


Expand All @@ -366,7 +359,7 @@ read_by_format <- function(file_to_load) {
find_path <- function(file_paths) {

extensions <- fs::path_ext(file_paths)
ext_order <- c("qs", "fst", "rds", "dta")
ext_order <- c("qs2", "fst", "rds", "dta")

f <- FALSE
i <- 1
Expand Down
14 changes: 12 additions & 2 deletions R/load_dlw_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#' verbose = FALSE)
#'
#' # Using id_name
#' load_dlw_data(id_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG.qs")
#' load_dlw_data(id_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG.qs2")
#'
#' # without ext also works
#' load_dlw_data(id_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG")
Expand Down Expand Up @@ -240,7 +240,7 @@ find_dlw_data <- function(
#' @param id_name id name of dlw data
#'
#' @returns character with id_name
#' @keywords internal
#' @export
#' @rdname load_dlw_data
#'
#' @examples
Expand All @@ -266,7 +266,9 @@ check_dlw_id_name <- \(id_name) {
#' @export
#'
#' @examples
#' \dontrun{
#' load_dlw_gmd_inventory()
#' }
load_dlw_gmd_inventory <- \() {
binv <- pipfun::get_pip_folders(folder = "dlw_inventory", verbose = FALSE)

Expand All @@ -289,7 +291,9 @@ load_dlw_gmd_inventory <- \() {
#' @export
#'
#' @examples
#' \dontrun{
#' load_dlw_gmd_log()
#' }
load_dlw_gmd_log <- \() {
binv <- pipfun::get_pip_folders(folder = "dlw_inventory", verbose = FALSE)

Expand All @@ -311,7 +315,9 @@ load_dlw_gmd_log <- \() {
#' @export
#'
#' @examples
#' \dontrun{
#' load_gmd_valid_inv()
#' }
load_gmd_valid_inv <- \() {
binv <- pipfun::get_pip_folders(folder = "dlw_metadata", verbose = FALSE)

Expand All @@ -333,7 +339,9 @@ load_gmd_valid_inv <- \() {
#' @export
#'
#' @examples
#' \dontrun{
#' load_gmd_valid_log()
#' }
load_gmd_valid_log <- \() {
binv <- pipfun::get_pip_folders(folder = "dlw_metadata", verbose = FALSE)

Expand All @@ -355,7 +363,9 @@ load_gmd_valid_log <- \() {
#' @export
#'
#' @examples
#' \dontrun{
#' load_gmd_valid_report()
#' }
load_gmd_valid_report <- \() {
binv <- pipfun::get_pip_folders(folder = "dlw_metadata", verbose = FALSE)

Expand Down
27 changes: 20 additions & 7 deletions R/load_pip_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@
#' verbose = FALSE)
#'
#' # Using id_name
#' load_pip_data(id_name = "LCA_2015_SLCHBS_INC_GPWG")
#'
#' }
#' load_pip_data(id_name = "BOL_2022_EH_INC_ALL")
#'}
load_pip_data <- function(
country_code = NULL,
surveyid_year = NULL,
Expand Down Expand Up @@ -159,10 +158,10 @@ load_pip_data <- function(
#' verbose = FALSE)
#'
#' # Find data
#' find_pip_data(country_code = "HRV")
#' find_pip_data(country_code = "BOL")
#'
#' # Latest year in EACH module
#' find_pip_data(country_code = "HRV", latest_year = TRUE)
#' find_pip_data(country_code = "BOL", latest_year = TRUE)
#' }
find_pip_data <- function(
latest_year = FALSE,
Expand Down Expand Up @@ -228,7 +227,13 @@ find_pip_data <- function(
#' @export
#'
#' @examples
#' \dontrun{
#' lr <- pipfun::get_latest_pip_release()
#' pipfun::setup_working_release(release = lr$release,
#' identity = lr$identity,
#' verbose = FALSE)
#' load_pip_release_inventory()
#' }
load_pip_release_inventory <- \(
version = NULL,
verbose = getOption("pipload.verbose"),
Expand Down Expand Up @@ -261,7 +266,13 @@ load_pip_release_inventory <- \(
#' @export
#'
#' @examples
#' \dontrun{
#' lr <- pipfun::get_latest_pip_release()
#' pipfun::setup_working_release(release = lr$release,
#' identity = lr$identity,
#' verbose = FALSE)
#' load_pip_master_inventory()
#' }
load_pip_master_inventory <- \(
format = "qs2",
version = NULL,
Expand Down Expand Up @@ -299,12 +310,14 @@ load_pip_master_inventory <- \(
#' @keywords internal
#'
#' @examples
#' check_pip_id_name("AGO_2000_HBS_CON_GPWG.qs")
#' \dontrun{
#' check_pip_id_name("AGO_2000_HBS_CON_GPWG.qs2")
#' check_pip_id_name("AGO_2000_HBS_CON_GPWG")
#' }
check_pip_id_name <- \(id_name) {
id_name <- id_name |>
fs::path_ext_remove() |>
fs::path(ext = "qs")
fs::path(ext = "qs2")

ptt <- get_from_piploadenv("pip_name_pattern")

Expand Down
4 changes: 2 additions & 2 deletions R/pip_load_all_aux.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ pip_load_all_aux <- function(replace = NULL,
purrr::map2(.x = aux_dirs,
.y = aux_indicators,
.f = ~{
fqs <- fs::path(.x, .y, ext = "qs")
fqs2 <- fs::path(.x, .y, ext = "qs2")
ffst <- fs::path(.x, .y, ext = "fst")
frds <- fs::path(.x, .y, ext = "rds")

f_exists <- purrr::map_lgl(c(ffst, frds, fqs), fs::file_exists)
f_exists <- purrr::map_lgl(c(ffst, frds, fqs2), fs::file_exists)
any(f_exists)

})
Expand Down
4 changes: 4 additions & 0 deletions R/pip_merge_aux.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
#' @export
#'
#' @examples
#' \dontrun{
#' pip_merge_aux()
#' }
pip_merge_aux <- function(tables = c("cpi", "ppp"),
ppp_year = 2017,
branch = c("DEV", "PROD", "main"),
Expand Down Expand Up @@ -98,7 +100,9 @@ pip_merge_aux <- function(tables = c("cpi", "ppp"),
#' @export
#'
#' @examples
#' \dontrun{
#' aux_ids()
#' }
aux_ids <- function(tables = NULL) {

l <- list()
Expand Down
2 changes: 2 additions & 0 deletions man/aux_ids.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions man/load_dlw_data.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 18 additions & 5 deletions man/load_pip_data.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/pip_load_aux.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions man/pip_merge_aux.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 9 additions & 7 deletions vignettes/read_write_pins.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,26 @@ Below are some real-world examples of how you might use these higher-level funct
```{r load_dlw_data}
# Get the latest release and set up the working environment
lr <- pipfun::get_latest_pip_release()
pipfun::setup_working_release(release = lr$release,
identity = lr$identity,
verbose = FALSE)
pipfun::setup_working_release(
release = lr$release,
identity = lr$identity,
verbose = FALSE
)

# Using pin_name (with extension)
load_dlw_data(pin_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG.qs") |>
load_dlw_data(pin_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG.qs2") |>
head()

# Using pin_name (without extension also works)
load_dlw_data(pin_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG") |>
load_dlw_data(pin_name = "HRV_2011_EU-SILC_V01_M_V04_A_GMD_GPWG") |>
head()

# Using country and year
load_dlw_data(country_code = "HRV", year = 2011) |>
load_dlw_data(country_code = "HRV", year = 2011) |>
head()

# Find all data for a country
find_dlw_data(country_code = "HRV") |>
find_dlw_data(country_code = "HRV") |>
head()

# Find the latest year in EACH module for a country
Expand Down