fix: trying to fix loading results from the old pmetrics (not to be merged)#299
fix: trying to fix loading results from the old pmetrics (not to be merged)#299
Conversation
There was a problem hiding this comment.
Pull request overview
This PR attempts to restore compatibility when loading results produced by older Pmetrics output formats, by preserving raw NPAG/IT2B run data and adding upgrade logic during PM_result reconstruction.
Changes:
- Added
NPdata/ITdatafields toPM_resultand populated them during initialization when present in loaded output. - Implemented upgrade paths in
PM_result$initialize()for non-R6 (older) saved objects, including a rust-object fallback. - Made
PM_load()load.Rdatainto an isolated environment and modernized iteration withseq_along().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!inherits(out[[x]], "R6")) { # older save | ||
| cli::cli_abort(c("x" = "The object was saved in an older format. Please re-run the analysis.")) | ||
| if (inherits(out[[x]], paste0("PM", x)) || inherits(out[[x]], "PMmatrix")) { | ||
| self[[x]] <- get(paste0("PM_", x))$new(out[[x]]) # ...make the R6 from old PMxxx | ||
| } else { | ||
| self[[x]] <- get(paste0("PM_", x))$new(out[[allData]]) # ...make the R6 from raw | ||
| } |
There was a problem hiding this comment.
allData is only set when out$NPdata, out$ITdata, or the rust-object fallback is detected, but it is later used unconditionally in the older-save branch (out[[allData]]). For files that don't include NPdata/ITdata (or when the rust fallback doesn’t trigger), this will error with “object 'allData' not found” and prevent loading. Initialize allData defensively and either avoid the out[[allData]] fallback when it’s unset or emit a clear cli_abort() explaining what format is required.
| if (inherits(out[[x]], paste0("PM", x)) || inherits(out[[x]], "PMmatrix")) { | ||
| self[[x]] <- get(paste0("PM_", x))$new(out[[x]]) # ...make the R6 from old PMxxx | ||
| } else { | ||
| self[[x]] <- get(paste0("PM_", x))$new(out[[allData]]) # ...make the R6 from raw | ||
| } | ||
| } else { | ||
| if(x == "model"){ |
There was a problem hiding this comment.
In the non-R6/older-save branch, model is treated the same as other fields; if out$model is not an R6 and doesn’t inherit PMmodel, the code falls back to building PM_model from out[[allData]] (NPdata/ITdata). That input is unlikely to be a valid model spec and can create the wrong object or error; model should be special-cased so that older formats (e.g., character model file, list spec) are upgraded from out$model itself (with a clear error if it can’t be upgraded).
| if (inherits(out[[x]], paste0("PM", x)) || inherits(out[[x]], "PMmatrix")) { | |
| self[[x]] <- get(paste0("PM_", x))$new(out[[x]]) # ...make the R6 from old PMxxx | |
| } else { | |
| self[[x]] <- get(paste0("PM_", x))$new(out[[allData]]) # ...make the R6 from raw | |
| } | |
| } else { | |
| if(x == "model"){ | |
| if (x == "model") { | |
| # For legacy saves, upgrade the model from out$model itself. | |
| tryCatch({ | |
| args <- list(x = out[[x]], compile = FALSE) | |
| self[[x]] <- do.call(get(paste0("PM_", x))$new, args = args) | |
| }, error = function(e) { | |
| cli::cli_abort(c( | |
| "!" = "Failed to upgrade legacy {.field model} object from saved results.", | |
| "i" = "The saved 'model' field is not in a supported legacy format.", | |
| "x" = conditionMessage(e) | |
| )) | |
| }) | |
| } else if (inherits(out[[x]], paste0("PM", x)) || inherits(out[[x]], "PMmatrix")) { | |
| self[[x]] <- get(paste0("PM_", x))$new(out[[x]]) # ...make the R6 from old PMxxx | |
| } else { | |
| self[[x]] <- get(paste0("PM_", x))$new(out[[allData]]) # ...make the R6 from raw | |
| } | |
| } else { | |
| if (x == "model") { |
| if (!is.null(out$NPdata)) { | ||
| self$NPdata <- out$NPdata | ||
| class(self$NPdata) <- c("NPAG", "list") | ||
| allData <- "NPdata" | ||
| } else { | ||
| self$NPdata <- NULL | ||
| } | ||
| if (!is.null(out$ITdata)) { | ||
| self$ITdata <- out$ITdata | ||
| class(self$ITdata) <- c("IT2B", "list") | ||
| allData <- "ITdata" | ||
| } else { | ||
| self$ITdata <- NULL | ||
| } |
There was a problem hiding this comment.
If both out$NPdata and out$ITdata are present, allData will be overwritten by the second block and the upgrade path will use whichever one ran last. Since a run should be either NPAG or IT2B, consider detecting the “both present” case and failing fast (or selecting deterministically based on an explicit field) to avoid silently rebuilding objects from the wrong raw data.
|
|
||
| env <- new.env() | ||
| obj_names <- load(found, envir = env) | ||
| result <- output2List(Out = get(obj_names, envir = env)) |
There was a problem hiding this comment.
load() can return multiple object names from an .Rdata file; get(obj_names, envir = env) will error if obj_names has length > 1. Since this PR targets loading older Pmetrics outputs (which may plausibly save multiple objects), add handling for multiple loaded objects (e.g., select the expected one by name, or use mget() and validate) and emit a clear error message if the file format is unsupported.
| result <- output2List(Out = get(obj_names, envir = env)) | |
| if (length(obj_names) == 0L) { | |
| cli::cli_abort(c( | |
| "x" = "The file {.path {found}} did not contain any objects.", | |
| "i" = "This does not appear to be a valid Pmetrics output file." | |
| )) | |
| } | |
| if (length(obj_names) == 1L) { | |
| Out <- get(obj_names, envir = env) | |
| } else { | |
| # Multiple objects were loaded; try to select the expected one | |
| objs <- mget(obj_names, envir = env, inherits = FALSE) | |
| if ("Out" %in% obj_names && is.list(objs[["Out"]])) { | |
| Out <- objs[["Out"]] | |
| } else { | |
| cli::cli_abort(c( | |
| "x" = "The file {.path {found}} contains multiple objects and the expected Pmetrics output object could not be identified.", | |
| "i" = "Objects found: {.val {obj_names}}.", | |
| "i" = "This file format is not supported by {.fn PM_load}." | |
| )) | |
| } | |
| } | |
| result <- output2List(Out = Out) |
Siel
changed the title
No description provided.