Fix as.named.vector.table() undefined-variable crash - #72
Conversation
The lines that build the return value `v` were commented out, so every call errored with "object 'v' not found". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b360c679d
ℹ️ 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".
| # names(v) <- dimnames(table)[[1]] | ||
| # Even after unclass(), the dim attribute remains, and is.vector() only returns TRUE if | ||
| # an object has no attributes other than names. | ||
| v <- as.vector(unclass(table)) |
There was a problem hiding this comment.
- Problem:
as.named.vector.table()documents...as arguments foras.vector(), but this call ignores them. - Trigger: A call such as
as.named.vector.table(x, mode = "character")silently returns integer counts instead of character values. - Fix: Pass
...toas.vector(unclass(table), ...).
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
|
could you please adress: " Forward ... to as.vector() |
|
Claude finished @vertesy's task in 46s —— View job Done
Addressed the Codex P2 finding: v <- as.vector(unclass(table), ...)Now calls like Pushed as commit Not addressed (out of scope for this request): the P1 suggestion to bump |
The function documents ... as arguments for as.vector() but previously ignored them, so calls like as.named.vector.table(x, mode = "character") silently returned integer counts instead of character values. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com>
Summary
Fixes the 1 bug flagged during the earlier code-annotation pass (#62), in
R/CodeAndRoll2.R. Re-opening againstdev— the previous attempt (#67) was closed without merging, targeted atmain.Why
as.named.vector.table()(deprecated in favor ofc()) never actually built its return valuev— the lines that construct it were commented out, so every call errored with "object 'v' not found". Restored the commented-out build logic.Verified interactively via
devtools::load_all().