Skip to content

intermingle.cbind: input validation, row-name alignment, safe subsetting, loop fix, docs - #38

Merged
vertesy merged 2 commits into
mainfrom
codex/update-intermingle.cbind-function
Aug 28, 2026
Merged

intermingle.cbind: input validation, row-name alignment, safe subsetting, loop fix, docs#38
vertesy merged 2 commits into
mainfrom
codex/update-intermingle.cbind-function

Conversation

@vertesy

@vertesy vertesy commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Harden intermingle.cbind() so inputs are validated, rows are matched by name (not position), and column iteration/subsetting is safe to avoid accidental dimension drops.

Description

  • Add a compact stopifnot() that asserts (is.data.frame(df1) || is.matrix(df1)), (is.data.frame(df2) || is.matrix(df2)), non-null rownames(df1) and rownames(df2), and ncol(df1) == ncol(df2).
  • When row counts differ, subset both df1 and df2 to the intersect(rownames(df2), rownames(df1)) using drop = FALSE to preserve dimensions.
  • When row counts match, explicitly reorder df2 to rownames(df1) with df2 <- df2[CommonGenes, , drop = FALSE] so rows are combined by name, not by position.
  • Replace for (x in 1:(2 * length(df1))) with for (x in seq_len(2L * ncol(df1))), update the roxygen @return to state a matrix is returned, regenerate man/intermingle.cbind.Rd, and bump Development/config.R version to 2.8.1.

Testing

  • Ran git diff --check which reported no issues and used git diff --stat to inspect changes; commit created successfully.
  • Attempted Rscript -e 'roxygen2::roxygenise(load_code = roxygen2::load_source)' but Rscript is not available in the environment so automated doc generation failed.
  • Man page man/intermingle.cbind.Rd was updated manually to match the new @return text and committed.
  • Verified working tree with git status --short and inspected the diffs before committing.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c79fe148e

ℹ️ 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".

Comment thread R/CodeAndRoll2.R
Comment thread Development/config.R
@vertesy

vertesy commented Aug 28, 2026

Copy link
Copy Markdown
Owner Author

@copilot

Could you please address and validate the following issue? And if you agree with it, please fix it.

"Problem: intermingle.cbind() treats equal row counts as proof that every df1 row name exists in df2, so the result can contain missing df2 values or fail during matrix subsetting instead of returning matched rows. Trigger: df1 and df2 have the same number of rows but different row-name sets. Fix: use the intersection path whenever the row-name sets differ, or reject unequal sets before reordering."

Co-authored-by: vertesy <5101911+vertesy@users.noreply.github.com>

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

@copilot

Could you please address and validate the following issue? And if you agree with it, please fix it.
...

Addressed in a2054f1. I agreed with the issue and changed intermingle.cbind() to use the intersection/subset path whenever row-name sets differ, even if row counts are equal. Validation: git diff --check passed; secret scan passed; automated code review found no comments; CodeQL had no analyzable code for this R-only change.

@vertesy
vertesy merged commit 1d43261 into main Aug 28, 2026
@vertesy
vertesy deleted the codex/update-intermingle.cbind-function branch August 28, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants