Problem
dft_rast_classify() calls terra::set.cats(x, ...) on the raster it was given (R/dft_rast_classify.R:49). set.cats() works in place, so the caller's object turns into a factor named class_name. When remap = NULL, x is the caller's own SpatRaster, not a copy.
r <- terra::rast(system.file("extdata", "example_2017.tif", package = "drift"))
names(r); terra::is.factor(r) # "data" FALSE
cl <- dft_rast_classify(list("2017" = r), source = "io-lulc")
names(r); terra::is.factor(r) # "class_name" TRUE <- the input changedFound in #81. A test built c(r17, r23) from the raw tiles after an earlier test had classified r17, and the layer names were no longer the file's. Any caller that classifies a raster and then reuses the original for anything that reads names or levels is affected, for example stacking it or passing it back into a function that validates it.
CLAUDE.md's spatial conventions already name the trap: set.cats() "mutates whatever raster it is given, so use it on a copy you own".
Fix (as landed, #93; revised from the original proposal)
The original proposal was to take a copy first, with x <- terra::deepcopy(x) or levels<-. That was not built, because it adds a second full copy: coltab<- already deep-copies as its first statement, at every supported terra (1.8-10 floor and 1.9.50). The shipped fix swaps the two setters instead. coltab<- runs first, and set.cats() then acts on that copy. The output is identical() to 0.19.0.
On BULK in memory (169M cells), kernel max RSS was 4.21 GiB for both the old code and the reorder, and 5.46 GiB for the deepcopy() proposal. A test checks that the caller is unmodified for file-backed, in-memory and list-element inputs. It is red on the old order, and it goes red again if terra ever makes coltab<- work in place.
I checked the other set.cats() call sites in R/, as the issue asked. Every one acts on a raster its function created (arithmetic, ifel, app(), rast(list), a rast() template, or strip_copy() after coltab<-), so none needed a change.
Found at scale and filed separately as #91: a raster that is already a factor gets empty levels, because terra::unique() returns labels.
Problem
dft_rast_classify()callsterra::set.cats(x, ...)on the raster it was given (R/dft_rast_classify.R:49).set.cats()works in place, so the caller's object turns into a factor namedclass_name. Whenremap = NULL,xis the caller's own SpatRaster, not a copy.Found in #81. A test built
c(r17, r23)from the raw tiles after an earlier test had classifiedr17, and the layer names were no longer the file's. Any caller that classifies a raster and then reuses the original for anything that reads names or levels is affected, for example stacking it or passing it back into a function that validates it.CLAUDE.md's spatial conventions already name the trap:
set.cats()"mutates whatever raster it is given, so use it on a copy you own".Fix (as landed, #93; revised from the original proposal)
The original proposal was to take a copy first, with
x <- terra::deepcopy(x)orlevels<-. That was not built, because it adds a second full copy:coltab<-already deep-copies as its first statement, at every supported terra (1.8-10 floor and 1.9.50). The shipped fix swaps the two setters instead.coltab<-runs first, andset.cats()then acts on that copy. The output isidentical()to 0.19.0.On BULK in memory (169M cells), kernel max RSS was 4.21 GiB for both the old code and the reorder, and 5.46 GiB for the
deepcopy()proposal. A test checks that the caller is unmodified for file-backed, in-memory and list-element inputs. It is red on the old order, and it goes red again if terra ever makescoltab<-work in place.I checked the other
set.cats()call sites inR/, as the issue asked. Every one acts on a raster its function created (arithmetic,ifel,app(),rast(list), arast()template, orstrip_copy()aftercoltab<-), so none needed a change.Found at scale and filed separately as #91: a raster that is already a factor gets empty levels, because
terra::unique()returns labels.