Follow-up to #252. The narrow fix for #252 added an inline vec_as_location(..., missing = "error") and an inform-dedup to [.tf_mv/tf_evaluate.tf_mv, but the matrix-i decomposition, the j-normalization, and the missing(j) default branch are duplicated across R/brackets.R ([.tf) and R/brackets-mv.R ([.tf_mv).
The reviewer of PR for #252 confirmed: "[.tf interleaves matrix-i decomp, vec_as_location, the missing(j) default branch, j-normalisation, and NA-fill on shared locals; a clean shared helper is genuinely invasive."
This issue tracks the extraction. Suggested shape:
# R/brackets.R
tf_validate_i <- function(i, x_size) {
vec_as_location(i, n = x_size, missing = "error")
}
tf_normalize_j <- function(j, arg, ...) {
# handle matrix-i, missing-j, value vs position semantics
}
Used by both [.tf and [.tf_mv. After extraction, the inline workaround in [.tf_mv can be removed and replaced with a call.
Important non-regression criterion (user explicitly said so during review): the four-mode behavior of [.tf (x[i] → tf, x[i, j] → matrix, x[i, j, matrix = FALSE] → list of data frames, x[cbind(i, j)] → numeric) is pet-design and intentional, baked in via backwards compatibility, and must be preserved. The shared helpers must not collapse or simplify this dispatch — they are extraction-only.
Found while reviewing PR for #252.
Follow-up to #252. The narrow fix for #252 added an inline
vec_as_location(..., missing = "error")and an inform-dedup to[.tf_mv/tf_evaluate.tf_mv, but the matrix-idecomposition, thej-normalization, and themissing(j)default branch are duplicated acrossR/brackets.R([.tf) andR/brackets-mv.R([.tf_mv).The reviewer of PR for #252 confirmed: "
[.tfinterleaves matrix-i decomp,vec_as_location, themissing(j)default branch,j-normalisation, and NA-fill on shared locals; a clean shared helper is genuinely invasive."This issue tracks the extraction. Suggested shape:
Used by both
[.tfand[.tf_mv. After extraction, the inline workaround in[.tf_mvcan be removed and replaced with a call.Important non-regression criterion (user explicitly said so during review): the four-mode behavior of
[.tf(x[i]→ tf,x[i, j]→ matrix,x[i, j, matrix = FALSE]→ list of data frames,x[cbind(i, j)]→ numeric) is pet-design and intentional, baked in via backwards compatibility, and must be preserved. The shared helpers must not collapse or simplify this dispatch — they are extraction-only.Found while reviewing PR for #252.