161 distributed fails at some nset values - #162
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a distributed-run edge case in stitchTogether() where subsetting a 1-column matrix could drop dimensions (turning it into a vector), causing loss of dimnames and downstream initialization failures (e.g., missing gene/sample names). It also adds a regression test and bumps the package version to reflect the bug fix.
Changes:
- Preserve matrix dimensions/dimnames during reordering in
stitchTogether()by subsetting withdrop = FALSE. - Add a test covering single-pattern distributed runs (both
genome-wideandsingle-cell) to prevent regression. - Bump package version from
3.33.1to3.33.2.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
R/DistributedCogaps.R |
Prevents 1-column matrices from being dropped to vectors during reordering in stitchTogether(). |
tests/testthat/test_DistributedCogaps.R |
Adds a regression test ensuring dimnames are retained for one-pattern distributed runs. |
DESCRIPTION |
Increments package version to reflect the bug fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
This pull request addresses a bug in the
stitchTogetherfunction that could cause loss of matrix dimension names when handling single-pattern distributed runs. The fix ensures that matrices retain their dimensions and names, preventing downstream errors. Additionally, a new test is added to verify this behavior. The package version is incremented to reflect the update.Bug fix for matrix dimension handling in distributed runs:
stitchTogetherfunction inR/DistributedCogaps.Rto usedrop=FALSEwhen subsetting matrices (Amean,Asd,Pmean,Psd). This prevents 1-column matrices from being converted to vectors, which previously resulted in the loss of row names and downstream errors. [1] [2]Testing improvements:
tests/testthat/test_DistributedCogaps.Rto ensure that running a distributed CoGAPS analysis with a single pattern retains matrix dimensions and row names, guarding against regression of this bug.Version update:
DESCRIPTIONfrom 3.33.1 to 3.33.2 to reflect the bug fix.