Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing targets for SpatRasterCollection: tar_terra_sprc() #50

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

njtierney
Copy link
Owner

* For use with multiple raster files, which are saved as a `SpatRasterCollection` object
* Tests (surprisingly?) Pass
* Need to address some issues of how GDAL options are used/passed/manipulated
* 🚀 🚀 🚀
@njtierney njtierney requested a review from Aariq April 3, 2024 23:26
Copy link
Collaborator

@Aariq Aariq left a comment

Choose a reason for hiding this comment

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

I'm fine with merging this as-is, especially if it is helping you get work done. However, what I really wish for is a way to use sprc() and terra::c() with dynamic and static branching. E.g., would the test pipeline in this PR perhaps be better done with tar_map() and a geotargets version of tarchetypes::tar_combine() like tar_combine_sprc()? Or would it be possible to create our own options for an iterate argument so one could use dynamic branching with iterate = "sprc" or iterate = "SpatRaster_layers"? I think we might have to dig more into the targets code or ask Will to answer those questions.

Comment on lines 130 to 150
.write_terra_rasters_sprc <- eval(
substitute(
function(object, path) {
for (i in seq(object)) {
if (i > 1) {
opt <- "APPEND_SUBDATASET=YES"
} else {
opt <- ""
}
terra::writeRaster(
x = object[i],
filename = path,
filetype = filetype,
overwrite = (i == 1),
gdal = opt
)
}
},
list(filetype = filetype, gdal = gdal)
)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it might be more useful to pull this out and create a write_sprc() function? Would it work on any SpatRasterCollection outside of a targets pipeline?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmmm interesting point! I'm not sure.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure if this is something we should add, in terms of reading/writing function outside of geotargets, we should let the targets manage that work.

@njtierney
Copy link
Owner Author

Re

I'm fine with merging this as-is, especially if it is helping you get work done. However, what I really wish for is a way to use sprc() and terra::c() with dynamic and static branching. E.g., would the test pipeline in this PR perhaps be better done with tar_map() and a geotargets version of tarchetypes::tar_combine() like tar_combine_sprc()? Or would it be possible to create our own options for an iterate argument so one could use dynamic branching with iterate = "sprc" or iterate = "SpatRaster_layers"? I think we might have to dig more into the targets code or ask Will to answer those questions.

I think that we should spin these issues out in to a separate issue. How all this works with static and dynamic branching is really important to consider, but I think we should approach those features separately.

@njtierney njtierney changed the title Implementing targets for SpatRasterCollection: tar_terra_rasts() Implementing targets for SpatRasterCollection: tar_terra_sprc() Apr 19, 2024
@njtierney njtierney merged commit 52603dd into master Apr 19, 2024
6 checks passed
@njtierney njtierney deleted the terra-rasts-sprc-3 branch April 19, 2024 04:57
@njtierney njtierney mentioned this pull request Apr 19, 2024
@Aariq Aariq mentioned this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Facilitate targets that are a list of geospatial rasters
2 participants