docs: fix text/documentation drift and broken examples (no behavior change) - #43
Merged
Conversation
…hange) Pure documentation and text fixes surfaced by a full repo audit, all verified to introduce no behavior change (R CMD check + a full sweep of every man/*.Rd example, 177/178 pass; the sole remaining failure is the pre-existing, already-tracked missing-@export bug in pU()). - README: fix a doc-generator bug where item #115 was mislabeled `which.max.multi()` (a private nested helper) while carrying get_max_colname_per_row()'s description, which was missing from the list entirely. - DESCRIPTION/config.R: reconcile stale "more than 130 productivity functions" blurb with the current true count (173, per the auto-generated function list). - Typo fixes: matrix.fromVector ("an columns" -> "and columns"), split_vec_to_list_by_N ("oggr" -> "or"). - mean_normalize: @description said "median", code and message say "mean" (copy-paste drift from the neighboring median_normalize). - as.list.df.by.row / as.list.df.by.col: @description texts were swapped relative to actual behavior. - symdiff: added the "not mathematically correct for >2 vectors" caveat (already printed at runtime) to the roxygen docs. - clip.outliers.at.percentile: `high` param doc corrected to state it is currently unused/reserved, since the function always clips both tails regardless of its value (a prior PR, caf4d60, already deliberately reverted an attempt to make it do something). - Removed dead code with zero functional purpose: a commented-out browser() call, a commented-out debug print, and two leftover interactive-session default arguments referencing undefined objects (UVI.assignment.filtered.3.HF, Sections.ls.Final). - Removed a stale @importFrom dplyr bind_rows tag (unused; the only reference is a commented-out line). - Fixed 5 broken @examples blocks found while verifying this PR (each is a documentation-only fix, not a behavior change): dput_pretty's example called the non-exported alias `pretty_dput()` instead of the exported `dput_pretty()`; list2fullDF.byNames's example used unnamed vectors where the function requires named ones; merge_1col_dfs_by_rn's example called the function with no arguments even though its only parameter has no default; splititsnames_byValues's example passed an unnamed vector into a function requiring named input; na.omit.mat's example called it directly, but roxygen2 registers it as an S3 method for na.omit() (its name matches the generic.class pattern) so it's only reachable via na.omit() dispatch, not a direct call. - Added 3 missing @PARAM entries (pFilter's `v`, pU's `head_n`, savehistory_2rstudio's `history_file`) that were causing an R CMD check WARNING. - Regenerated NAMESPACE/man/*.Rd via roxygen2 to match. Verification: R CMD build + R CMD check pass with only pre-existing, unrelated issues (S3-name-collision WARNING for na.omit.mat, missing base-package importFrom NOTEs, the 3 undeclared cross-package calls already tracked for separate PRs) — none introduced by this change. lintr::lint_package() reports ~1000 pre-existing style lints across the whole file (no .lintr config exists; the codebase predates lintr adoption per Development/Create_the_CodeAndRoll2_Package.R's own separate future styler::style_pkg() step) — none on the lines this PR touches are new regressions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b8c64cd20
ℹ️ 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".
…list Follow-up to the README fix: R/list.of.functions.in.CodeAndRoll2.det.md (the tracked, generator-produced function list that README.md's list is manually merged from, per Development/Create_the_CodeAndRoll2_Package.R) had the identical stale label at the same entry (#115). Fixed it to match, so the generated file and README stay consistent. The underlying generator bug (PackageTools::list_of_funs_to_markdown picking up the name of get_max_colname_per_row()'s private nested helper `which.max.multi` instead of the enclosing exported function) is in the PackageTools package, not available in this environment to fix directly -- this commit only re-syncs the two tracked text files that were already out of sync with each other.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
PR 1 of a batched repo audit (per the attached Repository Update Workflow Instructions). Pure documentation/text fixes — easiest category to review, zero behavior change. Full audit categorized ~40 problems; this PR covers the "text/documentation" bucket. Follow-up PRs (2-7) will cover undeclared-dependency crashes, a
1:length(x)style/edge-case sweep, and other logical bugs, one focused PR at a time.What's in this PR
which.max.multi()(a private nested helper, not public API) while carryingget_max_colname_per_row()'s real description, which was missing from the list entirely.matrix.fromVector("Swap rows an columns" → "and"),split_vec_to_list_by_N("A numeric oggr character vector" → "or").mean_normalize:@descriptionsaid "median"; the code and its own diagnostic message both say "mean" — copy-paste drift from the neighboringmedian_normalize. Fixed the doc.as.list.df.by.row/as.list.df.by.col: their@descriptiontexts were swapped relative to actual behavior (by-row said "by its columns" and vice versa).symdiff: added the "not mathematically correct for >2 vectors" caveat (already printed at runtime) to the roxygen docs so they match.clip.outliers.at.percentile: thehighparam is documented as "Clip above threshold? Default: TRUE" but is never referenced in the function body — it always clips both tails. Git history (caf4d60) shows a previous PR deliberately reverted an attempt to make it do something, so per discussion with the repo owner we're keeping current both-tail-clipping behavior and just fixing the doc to say the param is currently unused/reserved.browser()call, a commented-out debug print, and two leftover interactive-session default arguments referencing undefined objects (UVI.assignment.filtered.3.HF,Sections.ls.Final) that only worked by accident (R's lazy argument evaluation never forced them).@importFrom dplyr bind_rows(the only reference tobind_rowsis a commented-out line; the function usestibble::tibble()).Bonus: 5 broken
@examplesfound while verifying this PRRunning a full sweep of every
man/*.Rdexample (see Testing) surfaced 5 more documentation-only bugs — each is an example that doesn't reflect how the function actually needs to be called, not a behavior change:dput_pretty: example called the non-exported aliaspretty_dput(vec)instead of the exporteddput_pretty(vec)(the function is defined asdput_pretty <- pretty_dput <- function(vec) {...}, but onlydput_prettyis exported).list2fullDF.byNames: example used unnamed vectors (list(set.1 = LETTERS[1:5], ...)), but the function requires named vectors as list elements — its own default argument value already usesvec.fromNames(...)correctly, so the example was updated to match.merge_1col_dfs_by_rn: example called the function with no arguments even though its only parameter,list_of_dfs, has no default — replaced with a minimal working example.splititsnames_byValues: example passed an unnamed vector into a function that requires!is.null(names(namedVec)).na.omit.mat: example called it directly asna.omit.mat(mat), but its name matches R'sgeneric.classnaming convention, so roxygen2/R register it as an S3 method forna.omit()instead of a normal export — it's only reachable viana.omit(mat)afterclass(mat) <- c("mat", class(mat)). Fixed the example to show that. (The underlying "exported-but-unreachable-under-its-own-name" API quirk is a real, if minor, functional issue — left as a follow-up backlog item since fixing it properly means renaming or force-exporting, a behavior change, not a doc fix.)@paramentries (pFilter'sv,pU'shead_n,savehistory_2rstudio'shistory_file) that were causing anR CMD checkWARNING.All of the above were regenerated into
NAMESPACE/man/*.Rdviaroxygen2::roxygenise(".").Testing
R CMD build .+R CMD checkon the built tarball: passes with only pre-existing, unrelated issues left untouched by this PR — confirmed identical before/after this diff:S3 generic/method consistencyWARNING forna.omit.mat(the same S3-naming quirk noted above).llprint,FirstCol2RowNames,qbarplot— these are the 3 undeclared cross-package dependency crashes already scoped for PRs 2-4 of this batch.importFromentries (abline,hist,is,head,tail, etc.) — long-standing, unrelated to text/docs, tracked as a separate backlog item.man/*.Rdexamples individually (tools::Rd2ex+sys.source, package attached): 177/178 pass. The sole remaining failure ispU(), a pre-existing, already-known missing-@exportbug (confirmed via git history) intentionally left for its own dedicated PR since exporting a previously-unreachable function is a behavior/API change, not a doc fix.lintr::lint_package(): reports ~1000 pre-existing style lints (line-length, commented-code) across the whole file. No.lintrconfig exists in the repo, andDevelopment/Create_the_CodeAndRoll2_Package.Ritself calls out a separate, futurestyler::style_pkg()pass as the intended path to compliance — this codebase predates lintr/tidyverse-style adoption. None of the lines this PR touches introduce a new category of lint not already present nearby.testthat/automated tests added, per repo convention.Generated by Claude Code