Conversation
roxygen2's markdown mode (Roxygen: list(markdown = TRUE)) parses a bare `[Deprecated]` in an @title as a markdown reference-style link and silently converts it to \link{Deprecated} in the generated .Rd file - a broken link, since no help topic named "Deprecated" exists in this package. Confirmed by regenerating docs with roxygen2 and diffing against the checked-in .Rd files. Fix: use parentheses "(Deprecated)" instead of square brackets in both @title tags, which roxygen2's markdown parser leaves as plain text. Regenerated man/imovingSEM.Rd and man/movingAve2.Rd to confirm the fix (no \link{} in the output); NAMESPACE needed no change. Version bumped 2.8.15 -> 2.8.16 per repo convention for a real (if minor) doc-correctness fix. Note: R CMD check's dependency-availability step cannot complete in this sandbox because ReadWriter (a recently added hard Import) pulls in qs, which has a compile-time incompatibility with the stringfish version buildable here - a pre-existing environment limitation unrelated to this change. Verified instead via R CMD build (succeeds) plus direct roxygen2 regeneration/diffing as described above.
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>
Fix broken \link{} in movingAve2()/imovingSEM() @title tags
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>
pU() has a man/pU.Rd page and is documented in README's function index, but was missing @export, so it was never actually reachable as CodeAndRoll2::pU() (or bare pU() after library(CodeAndRoll2)) - including its own documented example, which calls it directly in a pipe. Fix: add @export tag, regenerate NAMESPACE (single new export(pU) line, alphabetically placed after export(pSee)). Verified by sourcing the function directly and running its own documented example (c(1,2,2,3,3,3) |> pU() |> sqrt()) - works and prints "3 unique elements: 1 2 3" as expected. Version bumped 2.8.15 -> 2.8.17 (distinct from sibling PR #71, which also branches from the same 2.8.15 base and already claimed 2.8.16). Note: R CMD check's dependency-availability step cannot complete in this sandbox due to a pre-existing, unrelated environment limitation (ReadWriter's dependency qs fails to compile against the available stringfish version here) - same issue already documented on PR #71, confirmed unrelated to this change. Verified via R CMD build (clean) plus direct functional testing as described above.
…ed-v Fix as.named.vector.table() undefined-variable crash
The version bump is now a decision the user makes explicitly; agents should not increment Development/config.R on their own, or ask for it in review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Never auto-bump the package version
getCategories(x) computed x[names(unique(x))], but base R's unique()
unconditionally strips names from atomic vectors - confirmed empirically
(names(unique(c(a=1,b=2))) is NULL). So names(unique(x)) is always NULL,
and indexing x[NULL] always returns an empty vector, regardless of input.
This broke the function 100% of the time, including its own documented
example (getCategories(c("A"=1,"B"=1,"C"=2,3)) returned named numeric(0)
instead of the intended "extract first occurrence of each unique value,
keeping its name" result.
Fix: use named_categ_vec[!duplicated(named_categ_vec)], matching the
already-correct sibling function unique.wNames() elsewhere in this file,
which uses the identical duplicated()-based pattern. Unlike unique(),
plain logical/numeric subsetting preserves names, and !duplicated()
selects first-occurrence positions directly rather than round-tripping
through unique()'s (name-losing) values.
Verified: the function's own documented example now returns the
correct c(A=1, C=2) result; also verified distinct-but-unnamed input
values (e.g. two different unnamed entries) are correctly kept as
separate entries rather than colliding, since this fix indexes by
position, not by the "" empty-string name that a naive
names(unique(x))-based fix would still collide on.
Version bumped 2.8.16 -> 2.8.18 (distinct from sibling PR #75, which
also branches from the same 2.8.16 base and already claimed 2.8.17).
…row/column selections Both subset operations, df[true_rownames, ] and df[, true_colnames], were missing drop = FALSE. Base R's default drop = TRUE collapses a data.frame subset to a plain vector whenever exactly one row or one column is selected, silently changing the return type from data.frame to vector for any caller who requests a single RowID or ColID. This also broke the function's own trailing Stringendo::iprint(dim(df)) diagnostic, which prints NULL for a vector instead of the actual dimensions. Fix: add drop = FALSE to both subset operations, matching the same fix already applied to other functions in this file (combine.matrices.by.rowname.intersect, merge_numeric_df_by_rn). Verified: selecting a single column (ColIDs = "a") and a single row (RowIDs = "r1") now both correctly return a data.frame with the expected dim() printed, instead of silently degrading to a vector; the normal multi-row/multi-col case is unaffected. Version bumped 2.8.16 -> 2.8.19 (distinct from sibling PRs #75/#76, which also branch from the same 2.8.16 base and already claimed 2.8.17/2.8.18).
rescale(vec, from, upto) computed vec - min(vec), then divided by max(vec) of that shifted vector. For a constant vector (or any single-element vector), the shifted vector is all zeros, so max(vec) is 0, and dividing by it silently produces Inf/NaN for every element instead of an error or a meaningful value - no warning is raised. Fix: detect the zero-range case (max == min) up front and return the midpoint of the target range, (from + upto) / 2, for all non-NA elements instead. This matches the well-established convention used by scales::rescale() for the identical degenerate-input case (verified: scales::rescale(c(5,5,5), to = c(0,100)) returns c(50,50,50)) - using an existing, widely-used package's behavior as the precedent avoids guessing a novel convention for this edge case. Verified: normal (non-degenerate) input is unaffected; constant vectors and single-element vectors now return the range midpoint instead of all-NaN; NA elements are left as NA rather than being assigned the midpoint; a custom target range on constant input scales correctly too. Version bumped 2.8.16 -> 2.8.20 (distinct from sibling PRs #75/#76/#77, which also branch from the same 2.8.16 base and already claimed 2.8.17/2.8.18/2.8.19).
…ble, for its default argument With transpose = TRUE (the default), the function built a 2-column (name, value) tibble, then did t(as.matrix(tbl)). as.matrix() on a mixed-type tibble coerces every column to a common type (character here, since one column is character), so the "transposed" result was always a character matrix - not a tibble as the function's own name and @description promise ("Convert a vector with names into a tibble"), and the original numeric type of the input vector was lost (e.g. 1 became "1"). Fix: use tibble::as_tibble_row(vec.w.names) for the transpose = TRUE branch - the standard tibble-package primitive for converting a named vector into a single-row, names-as-columns tibble, which preserves the input's original type. transpose = FALSE is unchanged (already returned a valid two-column tibble). Also removed a leftover commented-out alternative implementation of this same logic that was superseded by the current code before this fix. Verified: as_tibble_from_namedVec() (default args) now returns an actual tibble with the numeric type preserved, instead of a character matrix; as_tibble_from_namedVec(transpose = FALSE) is unaffected. Version bumped 2.8.16 -> 2.8.21 (distinct from sibling PRs #75/#76/#77/#78, which also branch from the same 2.8.16 base and already claimed 2.8.17-2.8.20).
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).
New PR-description rule: lead with a few bullets per major change (what was wrong / how it was fixed / behavior impact), scaled to the change's size, capped at 250 words -- split the PR instead of writing more. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The constant-input branch computed (from + upto) / 2, which can overflow to Inf before the division when from and upto are both large finite numbers (e.g. from = 1e308, upto = 1.1e308 - their sum exceeds .Machine$double.xmax, so the "midpoint" was Inf instead of the correct finite 1.05e308). Fix: compute the midpoint as from + (upto - from) / 2 instead - the subtraction of two same-magnitude numbers doesn't overflow, and adding a bounded increment to from doesn't either. Verified: the flagged overflow case now returns the correct finite midpoint (1.05e+308) instead of Inf; normal constant-input and non-degenerate cases are unchanged.
…he tibble fix tibble::as_tibble_row() defaults to .name_repair = "check_unique", which errors on a vector with duplicate names (e.g. c(a=1, a=2)) - a regression versus the previous (buggy in a different way) implementation, which never validated column-name uniqueness since names were just data values in a "name" column, not column headers. Fix: pass .name_repair = "minimal" to as_tibble_row(), which accepts the names as-is without validation, matching the previous implementation's permissiveness. Verified: as_tibble_from_namedVec(c(a=1, a=2)) now succeeds instead of erroring; the default and transpose = FALSE cases are unaffected.
…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.
Add missing @export to pU()
Require concise, structured PR descriptions
Replaces the per-file file.remove() call with a single call that matches every generated list.of.functions.in.*.det.md report by pattern (this repo generates 3: CodeAndRoll2, less.used, deprecated -- only the first was ever cleaned up before). The old line is commented out, not deleted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eanup Consolidate list-of-functions cleanup into one file.remove() call
Fix select_rows_and_columns() silently returning a vector for single row/column selections
…zero # Conflicts: # DESCRIPTION # Development/config.R Co-authored-by: vertesy <5101911+vertesy@users.noreply.github.com>
…pose Fix as_tibble_from_namedVec() returning a character matrix, not a tibble, for its default argument
Fix df.row.2.named.vector() always returning a list, never a vector
Fix getCategories() always returning an empty vector
Fix rescale() silent divide-by-zero on constant/single-element input
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b343ff3e7
ℹ️ 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".
| vmin <- min(vec, na.rm = TRUE) | ||
| vmax <- max(vec, na.rm = TRUE) | ||
| if (vmax == vmin) { | ||
| vec[!is.na(vec)] <- from + (upto - from) / 2 |
There was a problem hiding this comment.
Problem: rescale() returns Inf instead of the requested range midpoint. Trigger: This happens for a constant vec when from and upto are large finite values with opposite signs, such as -1e308 and 1e308, because upto - from overflows before division. Fix: Calculate the midpoint with an overflow-safe formula that handles opposite signs, such as halving both bounds before adding them.
Useful? React with 👍 / 👎.
Owner
Author
|
Could you please solve: "Avoid overflow for opposite-sign rescale bounds Problem: rescale() returns Inf instead of the requested range midpoint. Trigger: This happens for a constant vec when from and upto are large finite values with opposite signs, such as -1e308 and 1e308, because upto - from overflows before division. Fix: Calculate the midpoint with an overflow-safe formula that handles opposite signs, such as halving both bounds before adding them." |
Co-authored-by: vertesy <5101911+vertesy@users.noreply.github.com>
Contributor
Fixed in |
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.
No description provided.