Open fweber144 opened 1 year ago
I forgot to mention that other hidden reserved variables (other than .log_weight
) might be affected as well, so perhaps this needs to be resolved by a more general approach which ensures that subsetting columns keeps any hidden reserved variables.
I think, we need to make an explicit design decision on whether we want to retain hidden variables or not when subsetting via []
. I can imagine it can become very annoying for users if they try to get rid of all variables except for a view via []
but the hidden variables refuse to leave. @mjskay and @jgabry what do you think?
That said, you can always use subset_draws(., variable = ...)
to subset while retaining hidden variables.
Hmmm, that's a tough one. It does seem like the default behavior of [
should be to keep reserved variables (or at least .log_weight
--- which is currently equivalent since it's the only reserved variable for that format?) to avoid silent errors caused by unexpected dropping of weights. Maybe [
should gain an argument indicating whether reserved variables are subject to the subset (defaulting to not dropping them)? (Perhaps subset()
should too?)
I agree. So both [] and subset(_draws) should keep reserved variables by default but both gain an argument to turn this off? Sounds like a good approach to me.
I agree. So both [] and subset(_draws) should keep reserved variables by default but both gain an argument to turn this off? Sounds like a good approach to me.
Sounds good to me too.
One challenge towards this will be that we cannot just use NextMethod
for []
in some cases. For example, in the corresponding .draws_matrix method I see:
`[.draws_matrix` <- function(x, i, j, ..., drop = FALSE) {
# TODO: allow for argument 'reserved' as in '[.draws_df'
# right now this fails because NextMethod() cannot ignore arguments
out <- NextMethod("[", drop = drop)
if (length(dim(out)) == length(dim(x))) {
class(out) <- class(x)
.nchains <- nchains(x)
if (missing(i)) {
attr(out, "nchains") <- .nchains
} else if (.nchains > 1L) {
warn_merge_chains("index")
}
}
out
}
Just leaving this here for furture reference because I will surely forget otherwise.
I noticed that subsetting the variables of a
draws
object will drop weights:Created on 2023-08-15 with reprex v2.0.2
As you can see,
x_sub
lacks the hidden reserved variable.log_weight
.I'm not sure if this is intended behavior or not, but I think subsetting variables (variables only, not the draws or chains) should keep weights.