Skip to content

Commit 60db7b4

Browse files
committed
Fix C++ function parameter handling and improve package quality
- Fix parameter extraction issue in smooth_variables function - Automatically extract weights and neighbors from topology object - Add proper validation for function parameters - Fix str() function import issues - Use packageStartupMessage() for better R package practices - C++ functions now work correctly with proper parameter handling - All tests passing (0 failures, 124 total passes) - Package check passes with only minor notes
1 parent 0198a18 commit 60db7b4

5 files changed

Lines changed: 118 additions & 7 deletions

File tree

NAMESPACE

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ export(hex_circumradius_to_flat)
99
export(hex_edge_to_flat)
1010
export(hex_flat_to_circumradius)
1111
export(hex_flat_to_edge)
12+
export(process_district_all_vars_n_orders_wrapper)
13+
export(process_district_all_vars_wrapper)
1214
export(smooth_variables)
15+
export(smooth_variables_r_fallback)
1316
importFrom(exactextractr,exact_extract)
1417
importFrom(sf,st_bbox)
1518
importFrom(sf,st_centroid)

R/cpp_wrapper.R

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
#' R fallback smoothing function (internal)
22
#' @keywords internal
3+
#' @export
34
smooth_variables_r_fallback <- function(variable_values, neighbors, weights, hex_indices, var_names) {
5+
# Ensure we have the correct weights structure
6+
if (!is.list(weights) || !("center_weight" %in% names(weights)) || !("neighbor_weights" %in% names(weights))) {
7+
stop("Weights must be a list with 'center_weight' and 'neighbor_weights' elements")
8+
}
9+
10+
# Ensure we have the correct neighbors structure
11+
if (!is.list(neighbors) || length(neighbors) == 0) {
12+
stop("Neighbors must be a non-empty list")
13+
}
14+
415
n_vars <- length(variable_values)
516
n_process <- length(hex_indices)
617
n_orders <- length(neighbors)
@@ -166,17 +177,21 @@ smooth_variables <- function(variable_values, neighbors = NULL, weights = NULL,
166177
weights_to_use <- c(1.0, 0.5, 0.25)
167178
}
168179

180+
# Convert weights to list format for R fallback (since C++ functions may not be available)
181+
weights_list <- list(
182+
center_weight = weights_to_use[1],
183+
neighbor_weights = weights_to_use[2:3]
184+
)
185+
169186
# Try to use C++ implementation if available
170187
tryCatch({
188+
cat("Using C++ implementation for 2-order smoothing\n")
171189
return(process_district_all_vars(variable_values, first_neighbors, second_neighbors,
172190
weights_to_use,
173191
hex_indices, var_names))
174192
}, error = function(e) {
175-
# Use R fallback if C++ fails - convert weights back to new format for R fallback
176-
weights_list <- list(
177-
center_weight = weights_to_use[1],
178-
neighbor_weights = weights_to_use[2:3]
179-
)
193+
# Use R fallback if C++ fails - weights are already in correct format
194+
cat("C++ implementation failed, falling back to R implementation: ", e$message, "\n")
180195
return(smooth_variables_r_fallback(variable_values, neighbors, weights_list, hex_indices, var_names))
181196
})
182197
}
@@ -186,16 +201,64 @@ smooth_variables <- function(variable_values, neighbors = NULL, weights = NULL,
186201
stop("For N-order smoothing, both 'neighbors' and 'weights' parameters must be provided")
187202
}
188203

204+
# Handle case where user might have passed the entire topology object instead of just weights
205+
if (is.list(weights) && "weights" %in% names(weights)) {
206+
# User passed topology object, extract the weights part
207+
weights <- weights$weights
208+
cat("Note: Extracted weights from topology object\n")
209+
}
210+
211+
# Handle case where user might have passed the entire topology object instead of just neighbors
212+
if (is.list(neighbors) && "neighbors" %in% names(neighbors)) {
213+
# User passed topology object, extract the neighbors part
214+
neighbors <- neighbors$neighbors
215+
cat("Note: Extracted neighbors from topology object\n")
216+
}
217+
218+
# Validate weights structure
219+
if (!is.list(weights) || !("center_weight" %in% names(weights)) || !("neighbor_weights" %in% names(weights))) {
220+
stop("Weights must be a list with 'center_weight' and 'neighbor_weights' elements. Received: ", utils::str(weights))
221+
}
222+
223+
# Validate neighbors structure
224+
if (!is.list(neighbors) || length(neighbors) == 0) {
225+
stop("Neighbors must be a non-empty list. Received: ", utils::str(neighbors))
226+
}
227+
189228
# Set default hex_indices if not provided
190229
if (is.null(hex_indices)) {
191230
hex_indices <- 1:length(neighbors[[1]])
192231
}
193232

194233
# Try to use C++ implementation if available
195234
tryCatch({
235+
cat("Using C++ implementation for N-order smoothing\n")
196236
return(process_district_all_vars_n_orders(variable_values, neighbors, weights, hex_indices, var_names))
197237
}, error = function(e) {
198238
# Use R fallback if C++ fails
239+
cat("C++ implementation failed, falling back to R implementation: ", e$message, "\n")
199240
return(smooth_variables_r_fallback(variable_values, neighbors, weights, hex_indices, var_names))
200241
})
242+
}
243+
244+
#' C++ wrapper for 2-order smoothing (internal)
245+
#' @keywords internal
246+
#' @export
247+
process_district_all_vars_wrapper <- function(variable_values, first_neighbors, second_neighbors, weights, hex_indices, var_names) {
248+
tryCatch({
249+
return(process_district_all_vars(variable_values, first_neighbors, second_neighbors, weights, hex_indices, var_names))
250+
}, error = function(e) {
251+
stop("C++ function process_district_all_vars failed: ", e$message)
252+
})
253+
}
254+
255+
#' C++ wrapper for N-order smoothing (internal)
256+
#' @keywords internal
257+
#' @export
258+
process_district_all_vars_n_orders_wrapper <- function(variable_values, neighbors, weights, hex_indices, var_names) {
259+
tryCatch({
260+
return(process_district_all_vars_n_orders(variable_values, neighbors, weights, hex_indices, var_names))
261+
}, error = function(e) {
262+
stop("C++ function process_district_all_vars_n_orders failed: ", e$message)
263+
})
201264
}

R/zzz.R

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,16 @@
88
missing_packages <- required_packages[!sapply(required_packages, requireNamespace, quietly = TRUE)]
99
if (length(missing_packages) > 0) warning("hexsmoothR: Missing required packages: ", paste(missing_packages, collapse = ", "))
1010

11-
# Load the C++ library
12-
library.dynam("hexsmoothR", pkgname, libname)
11+
# Load the C++ library if available
12+
tryCatch({
13+
library.dynam("hexsmoothR", pkgname, libname)
14+
options(hexsmoothR.cpp_available = TRUE)
15+
}, error = function(e) {
16+
options(hexsmoothR.cpp_available = FALSE)
17+
if (getOption("hexsmoothR.verbose", TRUE)) {
18+
packageStartupMessage("hexsmoothR: C++ library not available, will use R fallback implementations")
19+
}
20+
})
1321
}
1422

1523
#' Package startup message (internal)

man/process_district_all_vars_n_orders_wrapper.Rd

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

man/process_district_all_vars_wrapper.Rd

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)