Skip to content

Commit 2713b48

Browse files
committed
Fix short ID coordinate scaling and CRS propagation; add .tz_count utility; update tests for round-trip and parsing accuracy
1 parent 221f4d4 commit 2713b48

7 files changed

Lines changed: 148 additions & 23 deletions

R/backend_sequential.R

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,8 @@ inspire_grid_from_extent_internal <- function(
3434
)
3535
}
3636

37-
tz_count <- function(x) {
38-
n <- 0L
39-
x <- as.integer(x)
40-
while (x %% 10L == 0L && x != 0) {
41-
n <- n + 1L
42-
x <- x %/% 10L
43-
}
44-
n
45-
}
4637
make_ids <- function(x_llc, y_llc, cs, axis_order, epsg = 3035) {
47-
nzeros <- tz_count(cs)
38+
nzeros <- .tz_count(cs)
4839
div <- as.integer(10^nzeros)
4940
size_lbl <- if (cs >= 1000) paste0(cs / 1000, "km") else paste0(cs, "m")
5041

R/inspire_grid_from_ids_internal.R

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ inspire_grid_from_ids_internal <- function(
2626
}
2727

2828
# Parse IDs to get coords and validate consistency across all IDs
29-
grid_df <- inspire_id_to_coords(ids, as_sf = FALSE)
29+
# IMPORTANT: Check ... for 'crs' to allow user overrides for short IDs
30+
dots <- list(...)
31+
crs_in <- dots$crs
32+
33+
grid_df <- inspire_id_to_coords(ids, as_sf = FALSE, crs = crs_in)
3034
names(grid_df) <- c("crs", "cellsize", "Y_LLC", "X_LLC")
3135

3236
if (length(unique(grid_df$crs)) > 1) {

R/inspire_id_format.R

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,13 @@ inspire_id_format <- function(ids, crs = 3035, axis_order = "NE") {
8484
paste0(parsed$cellsize, "m")
8585
)
8686

87-
y_short <- parsed$y / parsed$cellsize
88-
x_short <- parsed$x / parsed$cellsize
87+
# Use robust divisor logic (10^tz_count) consistent with grid generation
88+
divisors <- vapply(parsed$cellsize, function(cs) {
89+
10^.tz_count(cs)
90+
}, FUN.VALUE = numeric(1))
91+
92+
y_short <- parsed$y / divisors
93+
x_short <- parsed$x / divisors
8994

9095
if (axis_order == "NE") {
9196
sprintf("%sN%.0fE%.0f", res_str, y_short, x_short)
@@ -123,8 +128,13 @@ inspire_id_format <- function(ids, crs = 3035, axis_order = "NE") {
123128
numeric_res <- as.numeric(gsub("k?m", "", parsed$res_str))
124129
cellsize_m <- ifelse(is_km, numeric_res * 1000, numeric_res)
125130

126-
y_long <- parsed$y * cellsize_m
127-
x_long <- parsed$x * cellsize_m
131+
# Use robust multiplier logic
132+
multipliers <- vapply(cellsize_m, function(cs) {
133+
10^.tz_count(cs)
134+
}, FUN.VALUE = numeric(1))
135+
136+
y_long <- parsed$y * multipliers
137+
x_long <- parsed$x * multipliers
128138

129139
sprintf("CRS%.0fRES%.0fmN%.0fE%.0f", crs, cellsize_m, y_long, x_long)
130140
}

R/inspire_id_to_coords.R

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,15 @@ parse_inspire_ids <- function(inspire, is_long, is_short) {
123123
}
124124

125125
parsed[short_indices, "cellsize"] <- res_to_m(parsed_short_ne$cellsize_str)
126-
parsed[short_indices, "y"] <- parsed_short_ne$y
127-
parsed[short_indices, "x"] <- parsed_short_ne$x
126+
127+
# Calculate multipliers to restore coordinates based on cellsize trailing zeros
128+
# This inverses the division logic used during grid generation
129+
multipliers <- vapply(parsed[short_indices, "cellsize"], function(cs) {
130+
10^.tz_count(cs)
131+
}, FUN.VALUE = numeric(1))
132+
133+
parsed[short_indices, "y"] <- parsed_short_ne$y * multipliers
134+
parsed[short_indices, "x"] <- parsed_short_ne$x * multipliers
128135
}
129136

130137
return(parsed)

R/utils.R

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,19 @@ validate_disk_compatibility <- function(output_type, dsn) {
372372
return(TRUE)
373373
}
374374

375+
#' Count trailing zeros
376+
#' @keywords internal
377+
#' @noRd
378+
.tz_count <- function(x) {
379+
n <- 0L
380+
x <- as.integer(x)
381+
while (!is.na(x) && x != 0L && x %% 10L == 0L) {
382+
n <- n + 1L
383+
x <- x %/% 10L
384+
}
385+
n
386+
}
387+
375388
#' Internal helper to write a grid chunk to disk (sf or flat file)
376389
#' @keywords internal
377390
#' @noRd

tests/testthat/test-inspire_grid_from_ids.R

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,99 @@ test_that("inspire_grid_from_ids_internal handles empty input", {
120120
"Input 'inspire' cannot be an empty vector."
121121
)
122122
})
123+
124+
test_that("Round-trip: grid from extent -> short IDs -> grid from IDs matches original", {
125+
# This test verifies that a grid can be correctly reconstructed from short IDs
126+
# when CRS is provided, fixing the coordinate scaling and CRS propagation bugs.
127+
128+
# Step 1: Create a small extent for testing
129+
test_extent <- st_bbox(
130+
c(xmin = 1000000, ymin = 1000000, xmax = 1030000, ymax = 1030000),
131+
crs = st_crs(TARGET_CRS)
132+
)
133+
134+
# Step 2: Generate the original grid from extent with short IDs
135+
grid_original <- inspire_grid_from_extent(
136+
grid_extent = test_extent,
137+
cellsize_m = CELLSIZE,
138+
crs = TARGET_CRS,
139+
output_type = "sf_polygons",
140+
id_format = "short"
141+
)
142+
143+
expect_s3_class(grid_original, "sf")
144+
expect_gt(nrow(grid_original), 0)
145+
expect_true("GRD_ID" %in% names(grid_original))
146+
147+
# Step 3: Extract the short IDs
148+
short_ids <- grid_original$GRD_ID
149+
150+
# Verify they are indeed short format (no "CRS" prefix)
151+
expect_true(all(!startsWith(short_ids, "CRS")))
152+
153+
# Step 4: Regenerate grid from short IDs WITH crs parameter
154+
# This should NOT produce a warning and should use the correct CRS
155+
grid_regenerated <- inspire_grid_from_ids_internal(
156+
short_ids,
157+
output_type = "sf_polygons",
158+
crs = TARGET_CRS
159+
)
160+
161+
# Step 5: Compare the two grids
162+
expect_s3_class(grid_regenerated, "sf")
163+
expect_equal(nrow(grid_regenerated), nrow(grid_original))
164+
expect_equal(st_crs(grid_regenerated), st_crs(grid_original))
165+
166+
# The regenerated grid should have the same cell positions
167+
# Sort both by short ID to ensure proper comparison
168+
grid_original_sorted <- grid_original[order(grid_original$GRD_ID), ]
169+
grid_regenerated_sorted <- grid_regenerated[order(grid_regenerated$id), ]
170+
171+
# Compare LLC coordinates (these should match exactly)
172+
expect_equal(
173+
grid_original_sorted$X_LLC,
174+
grid_regenerated_sorted$X_LLC,
175+
tolerance = 1e-6
176+
)
177+
expect_equal(
178+
grid_original_sorted$Y_LLC,
179+
grid_regenerated_sorted$Y_LLC,
180+
tolerance = 1e-6
181+
)
182+
183+
# Compare geometries (should be identical)
184+
geom_original <- st_geometry(grid_original_sorted)
185+
geom_regenerated <- st_geometry(grid_regenerated_sorted)
186+
187+
# Extract centroids and compare
188+
centroid_original <- st_coordinates(st_centroid(geom_original))
189+
centroid_regenerated <- st_coordinates(st_centroid(geom_regenerated))
190+
191+
expect_equal(
192+
centroid_original[, "X"],
193+
centroid_regenerated[, "X"],
194+
tolerance = 1e-6
195+
)
196+
expect_equal(
197+
centroid_original[, "Y"],
198+
centroid_regenerated[, "Y"],
199+
tolerance = 1e-6
200+
)
201+
})
202+
203+
test_that("Short IDs with explicit CRS parameter do not produce warnings", {
204+
# This test verifies that the CRS propagation fix works correctly
205+
short_ids <- c("10kmN100E100", "10kmN101E100")
206+
207+
# With CRS provided, there should be NO warning
208+
expect_no_warning(
209+
grid_with_crs <- inspire_grid_from_ids_internal(
210+
short_ids,
211+
output_type = "sf_polygons",
212+
crs = TARGET_CRS
213+
)
214+
)
215+
216+
expect_s3_class(grid_with_crs, "sf")
217+
expect_equal(st_crs(grid_with_crs), st_crs(TARGET_CRS))
218+
})

tests/testthat/test-inspire_id_to_coords.R

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,23 @@ test_that("inspire_id_to_coords works with short format IDs", {
3030
)
3131

3232
# Default output (dataframe)
33+
# 10km cells with short coords N349E444 scale to (3490000, 4440000) in meters
34+
# Because 10000m has 4 trailing zeros, multiplier = 10^4 = 10000
3335
parsed_df <- inspire_id_to_coords(short_id, crs = 3035)
3436
expect_s3_class(parsed_df, "data.frame")
3537
expect_equal(names(parsed_df), c("crs", "cellsize", "y", "x"))
3638
expect_equal(parsed_df$cellsize, 10000) # Check m conversion
37-
expect_equal(parsed_df$y, 349)
38-
expect_equal(parsed_df$x, 444)
39+
expect_equal(parsed_df$y, 3490000)
40+
expect_equal(parsed_df$x, 4440000)
3941

4042
# sf output
4143
parsed_sf <- inspire_id_to_coords(short_id, as_sf = TRUE, crs = 3035)
4244
expect_s3_class(parsed_sf, "sf")
4345
# Short format IDs default to CRS 3035
4446
expect_equal(sf::st_crs(parsed_sf), sf::st_crs(3035))
4547
coords <- sf::st_coordinates(parsed_sf)
46-
expect_equal(as.numeric(coords[1, "X"]), 444)
47-
expect_equal(as.numeric(coords[1, "Y"]), 349)
48+
expect_equal(as.numeric(coords[1, "X"]), 4440000)
49+
expect_equal(as.numeric(coords[1, "Y"]), 3490000)
4850
})
4951

5052
test_that("inspire_id_to_coords handles vectorization and mixed-format issues", {
@@ -60,10 +62,12 @@ test_that("inspire_id_to_coords handles vectorization and mixed-format issues",
6062
)
6163
expect_equal(nrow(parsed_vec), 3)
6264

65+
# 1km = 1000m has 3 trailing zeros, multiplier = 10^3 = 1000
66+
# So short coords 3618 and 4478 scale to 3618000 and 4478000
6367
expect_equal(parsed_vec$cellsize[1], 1000)
6468
expect_equal(parsed_vec$x[1], 4447000)
65-
expect_equal(parsed_vec$y[2], 3618)
66-
expect_equal(parsed_vec$x[3], 4478)
69+
expect_equal(parsed_vec$y[2], 3618000)
70+
expect_equal(parsed_vec$x[3], 4478000)
6771
})
6872

6973
test_that("inspire_id_to_coords handles edge cases: malformed, NA, and empty inputs", {

0 commit comments

Comments
 (0)