Fix df.row.2.named.vector() always returning a list, never a vector - #80
Conversation
df.row.2.named.vector() computed as.vector(df[row, , drop = TRUE]). Base R's drop = TRUE only lets a single-COLUMN selection simplify to an atomic vector; extracting one ROW across multiple columns never simplifies this way, regardless of drop, because a row spanning mixed-type columns can't automatically collapse to one atomic type. So df[row, , drop = TRUE] is always a one-row data.frame/tibble (list-like), and as.vector() on that just returns the equivalent list - not an atomic vector as the function's own name and @description promise ("Convert a dataframe row into a vector"). Confirmed this fails for plain data.frame input too, not only tibbles as originally suspected - the function never actually worked for its stated purpose, for any input. Fix: use unlist(df[row, , drop = TRUE], use.names = FALSE) instead of as.vector(...) - unlist() correctly flattens the one-row list-like object into a proper atomic vector (coercing to a common type across columns where needed, same as any other unlist() call on mixed-type data). Verified: returns a proper named atomic vector for both a plain data.frame and a tibble (previously both returned a list); the existing names = <column> feature (naming from a separate ID column) is unaffected. Version bumped 2.8.16 -> 2.8.22 (distinct from sibling PRs #75/#76/#77/#78/#79, which also branch from the same 2.8.16 base and already claimed 2.8.17-2.8.21).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f78d297b03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ger codes
unlist() silently replaces factor values with their underlying
integer level codes when combined with non-factor elements in the
same list (a well-known R gotcha - e.g.
unlist(list(factor("b", levels=c("a","b")), 5)) is c(2, 5), not
c("b","5")). Since df[row, , drop=TRUE] is a list of per-column
values, any factor column in a mixed-type row silently became its
integer code instead of its label (e.g. "control" became "1").
Fix: convert factor elements to character before unlist()-ing, so
their displayed labels are preserved instead of their internal
integer codes.
Verified: a data.frame row with a factor column mixed with a numeric
column now correctly returns the factor's label ("control") instead
of its integer code; all previously-verified scenarios (plain
data.frame, tibble, the names= column-source feature) are unaffected.
|
Could you please explain to me why in a function called I'm not sure if this PR is actually improving the code or breaking something, That was working well before. |
|
Claude finished @vertesy's task in 52s —— View job Todo
AnswerGood catch to double check, but this isn't a bug — the names aren't actually being dropped. Here's the current code ( df.row.2.named.vector <- function(df, row, names = NULL) {
stopifnot(length(row) == 1)
row_list <- lapply(df[row, , drop = TRUE], function(x) if (is.factor(x)) as.character(x) else x)
vec <- unlist(row_list, use.names = FALSE)
names(vec) <- if (is.null(names)) colnames(df) else as.vector(unlist(df[names]))
return(vec)
}Line 1052 builds the vector without names, but line 1053 immediately assigns the real names right after — either the data frame's Why not just use
So: the naming behavior you'd expect from the function name is preserved — it's just implemented via explicit assignment (line 1053) rather than via If you'd like, I can add a test case exercising a row with a list-column (or otherwise document this behavior) to make the intent explicit for future readers — just let me know. |
The bug
df.row.2.named.vector()computedas.vector(df[row, , drop = TRUE]). Base R'sdrop = TRUEonly lets a single-column selection simplify to an atomic vector; extracting one row across multiple columns never simplifies this way, regardless ofdrop, because a row spanning mixed-type columns can't automatically collapse to one atomic type. Sodf[row, , drop = TRUE]is always a one-row data.frame/tibble (list-like), andas.vector()on that just returns the equivalent list — not an atomic vector as the function's own name and@descriptionpromise ("Convert a dataframe row into a vector"). Confirmed this fails for plaindata.frameinput too, not only tibbles as originally suspected — the function never actually worked for its stated purpose, for any input.Fix
Use
unlist(df[row, , drop = TRUE], use.names = FALSE)instead ofas.vector(...)—unlist()correctly flattens the one-row list-like object into a proper atomic vector (coercing to a common type across columns where needed, same as any otherunlist()call on mixed-type data).Verification
data.frameand atibble(previously both returned alist).names = <column>feature (naming from a separate ID column) is unaffected.R CMD build .succeeds.2.8.16→2.8.22(distinct from sibling PRs Add missing @export to pU() #75/Fix getCategories() always returning an empty vector #76/Fix select_rows_and_columns() silently returning a vector for single row/column selections #77/Fix rescale() silent divide-by-zero on constant/single-element input #78/Fix as_tibble_from_namedVec() returning a character matrix, not a tibble, for its default argument #79, which also branch from the same2.8.16base and already claimed2.8.17-2.8.21).R CMD check's dependency-availability step cannot complete in this sandbox due to a pre-existing, unrelated environment limitation (ReadWriter's dependencyqsfails to compile against the availablestringfishversion here) — same issue already documented on the other open PRs in this batch, confirmed unrelated to this change.Generated by Claude Code