Skip to content

Commit b31cd9f

Browse files
Fix approach to extract rois regex to be more robust
1 parent 73d51fe commit b31cd9f

9 files changed

Lines changed: 107 additions & 79 deletions

NAMESPACE

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export(extract_bids_info)
1515
export(extract_rois)
1616
export(filtfilt_cpp)
1717
export(get_fmriprep_outputs)
18-
export(get_postproc_stream_outputs)
18+
export(get_postproc_output_files)
1919
export(get_project_status)
2020
export(get_step_title)
2121
export(get_subject_status)

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* Jobs now write a manifest of files and times to the job tracking database for more thorough completeness tests
99
* Added optional low-pass filtering of motion parameters, matching (Gratton)
1010
* Gracefully adjust motion filtering parameters if they fall above Nyquist at this TR
11+
* Modify extract ROIs config to avoid input_regex and always generate it internally from postproc stream
1112

1213
# BrainGnomes 0.7-5
1314

R/process_subject.R

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -638,11 +638,18 @@ submit_extract_rois <- function(
638638

639639
# pull the requested extraction stream from the broader list
640640
ex_cfg <- scfg$extract_rois[[ex_stream]]
641+
if (!is.null(ex_cfg$input_regex)) {
642+
msg <- "extract_rois/input_regex is ignored by run_project; inputs are derived from postprocess streams."
643+
if (!is.null(lg)) {
644+
to_log(lg, "warn", msg)
645+
} else {
646+
warning(msg, call. = FALSE)
647+
}
648+
}
641649
if (isTRUE(scfg$force)) ex_cfg$overwrite <- TRUE # enable overwrite of ROIs if force=TRUE
642650

643-
# Every extract_rois stream can pull for 1+ postprocess streams. Based on postprocess input stream(s), generate regular expressions
644-
# need to find outputs of postproc stream. A little tricky given that desc may not be in input_regex. This is handled inside extract_cli.R,
645-
# which runs once the job fires (and any expected files are now available from earlier processing stages)
651+
# Every extract_rois stream can pull for 1+ postprocess streams. Pass through the input spec
652+
# and bids_desc for each stream so extract_cli.R can target postprocessed outputs directly.
646653
ex_cfg$input_regex <- sapply(ex_cfg$input_streams, function(ss) scfg$postprocess[[ss]]$input_regex, USE.NAMES = FALSE)
647654

648655
# the bids_desc of the postprocess stream is used to update the matched files (to get the outputs of postprocessing)

R/setup_postprocess.R

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,51 +1428,43 @@ setup_apply_aroma <- function(ppcfg = list(), fields = NULL) {
14281428

14291429

14301430

1431-
#' Determine expected output files for a postprocessing stream
1431+
#' List postprocessed output files for a stream based on its input spec
14321432
#'
1433-
#' Given a directory of candidate input files and a specification for
1434-
#' matching those inputs, this function returns the full paths to the
1435-
#' corresponding postprocessed NIfTI files. The output filenames are
1436-
#' derived by replacing the `desc` entity in each input file with the
1437-
#' stream's `bids_desc` value.
1433+
#' Converts a postprocess input specification into a pattern that targets
1434+
#' postprocessed outputs by ensuring the `desc` entity matches `bids_desc`.
14381435
#'
1439-
#' @param input_dir Directory containing the input NIfTI files to be
1440-
#' postprocessed.
1441-
#' @param input_regex Specification used to match the input files. This may be
1442-
#' a space-separated set of BIDS entities (e.g., "desc:preproc suffix:bold")
1443-
#' or a regular expression prefixed with "regex:".
1444-
#' @param bids_desc The `desc` value to use for the output filenames.
1436+
#' @param input_dir Directory containing postprocessed outputs.
1437+
#' @param input_regex Specification used to match the input files for the stream.
1438+
#' May be a space-separated set of BIDS entities or a regex prefixed with "regex:".
1439+
#' @param bids_desc The `desc` value used for postprocessed outputs.
14451440
#'
1446-
#' @return A character vector of full paths to the expected postprocessed
1447-
#' NIfTI files. The vector is named with the corresponding input file.
1441+
#' @return A character vector of full paths to matching postprocessed outputs.
14481442
#' @export
1449-
#' @examples
1450-
#' \dontrun{
1451-
#' get_postproc_stream_outputs(
1452-
#' input_dir = "/path/to/subject",
1453-
#' input_regex = "desc:preproc suffix:bold",
1454-
#' bids_desc = "clean"
1455-
#' )
1456-
#' }
1457-
#' @importFrom checkmate assert_directory_exists assert_string
1458-
get_postproc_stream_outputs <- function(input_dir, input_regex, bids_desc) {
1443+
get_postproc_output_files <- function(input_dir, input_regex, bids_desc) {
14591444
checkmate::assert_directory_exists(input_dir)
14601445
if (is.null(input_regex)) input_regex <- "desc:preproc suffix:bold"
1461-
checkmate::assert_string(input_regex)
1446+
checkmate::assert_character(input_regex, min.len = 1L)
14621447
checkmate::assert_string(bids_desc)
14631448

1464-
pattern <- construct_bids_regex(input_regex)
1465-
in_files <- list.files(path = input_dir, pattern = pattern, recursive = TRUE, full.names = TRUE)
1449+
resolve_spec <- function(spec) {
1450+
spec <- trimws(spec)
1451+
if (grepl("^regex:", spec)) {
1452+
return(trimws(sub("^regex\\s*:", "", spec)))
1453+
}
14661454

1467-
if (length(in_files) == 0L) return(character(0))
1455+
if (grepl("\\bdesc:", spec)) {
1456+
spec <- sub("\\bdesc:[^[:space:]]+", paste0("desc:", bids_desc), spec)
1457+
} else {
1458+
spec <- paste(spec, paste0("desc:", bids_desc))
1459+
}
1460+
1461+
construct_bids_regex(spec)
1462+
}
14681463

1469-
out_files <- vapply(in_files, function(f) {
1470-
info <- as.list(extract_bids_info(f))
1471-
info$description <- bids_desc
1472-
construct_bids_filename(info, full.names = TRUE)
1473-
}, character(1))
1464+
patterns <- vapply(input_regex, resolve_spec, character(1))
1465+
files <- unlist(lapply(patterns, function(pat) {
1466+
list.files(path = input_dir, pattern = pat, recursive = TRUE, full.names = TRUE)
1467+
}), use.names = FALSE)
14741468

1475-
# leave as unnamed for now (don't need to remember input file)
1476-
# names(out_files) <- in_files
1477-
out_files
1469+
unique(files)
14781470
}

inst/extract_cli.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ if (!checkmate::test_directory(cfg$input)) {
6969
stop("No valid directory provided as --input")
7070
}
7171

72-
input_files <- get_postproc_stream_outputs(cfg$input, cfg$input_regex, cfg$bids_desc)
72+
input_files <- get_postproc_output_files(cfg$input, cfg$input_regex, cfg$bids_desc)
7373

7474
if (length(input_files) == 0L) {
7575
stop("Cannot find files for ROI extraction with --input: ", cfg$input)

man/get_all_nodes.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/get_postproc_output_files.Rd

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

man/get_postproc_stream_outputs.Rd

Lines changed: 0 additions & 38 deletions
This file was deleted.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
test_that("get_postproc_output_files targets postprocessed outputs", {
2+
tmp_dir <- tempfile("bg_postproc_")
3+
dir.create(tmp_dir, recursive = TRUE)
4+
5+
postproc <- file.path(
6+
tmp_dir,
7+
"sub-01_task-impressions_space-MNI152NLin2009cAsym_desc-postproc_clean_bold.nii.gz"
8+
)
9+
preproc <- file.path(
10+
tmp_dir,
11+
"sub-01_task-impressions_space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz"
12+
)
13+
file.create(postproc)
14+
file.create(preproc)
15+
16+
rx_with_desc <- "desc:preproc task:impressions space:MNI152NLin2009cAsym suffix:bold"
17+
res_with_desc <- get_postproc_output_files(tmp_dir, rx_with_desc, "postproc_clean")
18+
expect_true(postproc %in% res_with_desc)
19+
expect_false(preproc %in% res_with_desc)
20+
21+
rx_no_desc <- "task:impressions space:MNI152NLin2009cAsym suffix:bold"
22+
res_no_desc <- get_postproc_output_files(tmp_dir, rx_no_desc, "postproc_clean")
23+
expect_true(postproc %in% res_no_desc)
24+
expect_false(preproc %in% res_no_desc)
25+
})

0 commit comments

Comments
 (0)