Skip to content

Fix CV_Tissue merge loop producing duplicate columns and missing Anatomical_entity_name - #72

Merged
toniecrumley merged 23 commits into
mainfrom
devel
May 26, 2026
Merged

Fix CV_Tissue merge loop producing duplicate columns and missing Anatomical_entity_name#72
toniecrumley merged 23 commits into
mainfrom
devel

Conversation

@toniecrumley

@toniecrumley toniecrumley commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

There are several old changes for version numbers and such that I didn't realize were in here. Also, I should have branched off of devel and then merged back to it but I will do that right next time.

Fix CV_Tissue merge loop producing duplicate columns and missing Anatomical_entity_name

The original loop fed the accumulated result back into each merge iteration,
creating a Cartesian product with cascading .x/.y column duplicates and a
broken Anatomical_entity_name column (also contained a typo in the fallback
rename). This was rewritten to join each species independently against the original
id_dataframe and combine on all ID columns + Anatomical_entity_name.

Fixes #71

Author Checklist

  • This repository has a Lasseigne Lab ruleset configured.
    See instructions here.
  • A singularity container has been created with all dependancies and the
    cap_container command has been included in each script using a container.
  • All of the necessary scripts, dependencies, and inputs have been included
    in this pull request.
  • Only files relevant to this change are included; no temporary, generated,
    or unrelated files are part of this pull request. Use .gitignore to prevent
    irrelevant files from accidentally being included.
  • Results have been reproduced and verified with lasseignelab and
    default environments by running the job until two consecutive runs produce
    the same results. Code comments (per script) have been added for
    non-reproducible scripts. Include information about why the script is not
    reproducible.
  • All automated code checks have passed. The "All checks have passed"
    message should show in the GitHub pull request status box above the
    "Merge pull request" button.
  • No merge conflicts exist. Merge conflicts are indicated by the "This
    branch has conflicts that must be resolved" message in the GitHub pull
    request status box above the "Merge pull request" button.
  • Primary and secondary reviewers have been requested to review this
    pull request.

Self/Peer Review Checklist (Coding Guidelines)

  • Meaningful variable and function names
  • File header comments
  • Function comments
  • In-line comments summarize logical sections of code by concisely explaining
    why, not what the code is doing. Avoid excessive or redundant commenting.
  • Random seed is set
  • Reasonable processing time for each job or script is specified
  • All outputs have been saved
  • Session info included in the scripts
  • Cross-reference the working draft of the paper
  • Confirm the code performs the intended analysis
  • Confirm the code reproduces the reported results

CoSIA

Verify that the modifications to CoSIA work correctly.

Setup

cd $USER_SCRATCH
git clone git@github.com:lasseignelab/CoSIA.git
cd CoSIA
git checkout devel

Test

Run the testthat unit test suite:

./tests/test.sh

Validate that new and old CoSIA produce the same results for functionality that wasn't broken:

./validation/validate.sh

Cleanup

cd $USER_SCRATCH
rm -rf CoSIA

Worthey Lab

Verify that the modifications to CoSIA fix the issues identified by the Worthey Lab in the vignette_script_walkthrough_CPAM_All_Models_Together_Tissues.R script and that the results of that script are correct.

Setup

cd $USER_SCRATCH
git clone git@github.com:toniecrumley/CoSIA-worthey.git
cd CoSIA-worthey
git checkout vignette_walkthroughs_example_plot_creation_scripts

Test

Run the testthat unit test suite:

./tests/test.sh

Validate that new and old CoSIA produce the same results for functionality that wasn't broken:

./validation/validate.sh

Run the analysis scripts created by the Worthey Lab.

./analysis/test.sh

Cleanup

cd $USER_SCRATCH
rm -rf CoSIA-worthey

adc0032 and others added 16 commits October 18, 2024 15:19
… markers

- Add tests/testthat/ suite with 59 tests covering CoSIAn constructor,
  viewCoSIAn, getConversion, getGEx, and getGExMetrics (all 6 metric types)
- Fix viewCoSIAn: replace ifelse() with if/else so data frame slots are
  returned intact rather than coerced to a scalar
- Fix CV_Tissue in getGExMetrics: guard Anatomical_entity_name.x filter
  behind a column existence check so single-species calls no longer crash
- Remove PackageStatus: Deprecated from DESCRIPTION and .Deprecated() call
  from R/zzz.R; package remains active in Bioconductor 3.22
- Add Config/testthat/edition: 3 to DESCRIPTION
- Add CLAUDE.md and CLAUDE_CHANGELOG.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…omical_entity_name

The original loop fed the accumulated result back into each merge iteration,
creating a Cartesian product with cascading .x/.y column duplicates and a
broken Anatomical_entity_name column (also contained a typo in the fallback
rename). Rewritten to join each species independently against the original
id_dataframe and combine on all ID columns + Anatomical_entity_name.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@toniecrumley toniecrumley changed the title Devel Fix CV_Tissue merge loop producing duplicate columns and missing Anatomical_entity_name Apr 23, 2026
toniecrumley and others added 4 commits April 23, 2026 13:08
Three scripts in validation/ compare Bioconductor release against the dev
version to verify correctness of the CV_Tissue merge rewrite:
- 01_run_bioc.R: captures reference output from installed Bioconductor CoSIA
- 02_run_dev.R:  captures output from the modified dev version
- 03_compare.R:  compares results with structural checks, exact value
  comparison for single-species, and manual spot-check instructions

.Rbuildignore updated to exclude validation/, CLAUDE.md, CLAUDE_CHANGELOG.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Runs devtools::test() on push/PR to devel and main using the official
bioconductor/bioconductor_docker:RELEASE_3_22 container. Caches R
packages keyed to DESCRIPTION and ExperimentHub data (EH7858-EH7863)
with a static key to avoid redundant downloads across runs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…parison

1.11.1 is the Bioconductor production version with the CV_Tissue bug.
1.11.2 is the fixed release.

Validation strategy:
- CV_Tissue: confirm 1.11.1 crashes (bug), 1.11.2 succeeds (fix), with
  structural checks and cross-species consistency check on the fixed output
- CV_Species and DS_* metrics: unchanged code paths, so values must match
  exactly between versions

All scripts now cover all six metric types with a shared set of inputs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lizzyjoan lizzyjoan self-assigned this May 6, 2026
toniecrumley and others added 2 commits May 6, 2026 12:04
mkdir -p now runs unconditionally in ensure_sif so the bin/container
directory is created even when the SIF is already present.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@toniecrumley
toniecrumley marked this pull request as ready for review May 8, 2026 18:03
@lizzyjoan

lizzyjoan commented May 14, 2026

Copy link
Copy Markdown

Trying to run the testthat unit suite, I run into this, but haven't looked further into it yet

SIF not found: /scratch/lizzyr/CoSIA/bin/container/bioc_cosia_1.10.0.sif
Pulling from docker://lizzyr/bioc_cosia:1.10.0 ...
./tests/test.sh: line 16: singularity: command not found

**EDIT my bad-- did not realize test.sh was not a job, but runs interactively when I tried earlier. 😅 Rerunning on interactive node now!

@tsoelter tsoelter 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.

I think everything works as expected. I played around with the reproducible example, with the vignette, and the Shiny app (in addition to what is in the PR instructions) and did not encounter any issues.

@lizzyjoan lizzyjoan 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.

Everything looks good and worked with the different examples, tests, and seems to be as expected results!

@toniecrumley
toniecrumley merged commit 33abe9f into main May 26, 2026
2 checks passed
toniecrumley added a commit that referenced this pull request Jul 22, 2026
Fix CV_Tissue merge loop producing duplicate columns and missing Anatomical_entity_name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plotCVGEx fails with "Column Anatomical_entity_name doesn't exist"

6 participants