From 627950da31468b36a327c55a4adc3fd13342dc4c Mon Sep 17 00:00:00 2001 From: caleblareau Date: Wed, 8 Jul 2026 12:49:46 -0400 Subject: [PATCH 1/2] Fix CLI version regression, gzip barcode counting; add CI + tests - mgatk/__init__.py: resolve __version__ from installed metadata (was a stale hardcoded 0.7.1) so all entry points share one source of truth. - cli.py / clidel.py / clifind.py: restore module-level `from mgatk import __version__`; a bad merge left @click.version_option(version=__version__) at import time while __version__ was only bound inside main(), crashing every console script with NameError. - mgatkHelp.file_len: use the gzip-aware opener so gzipped barcode lists are counted correctly in tenx/check mode (previously raised UnicodeDecodeError). - tests: add test_version.py (all three CLIs report a version) and test_remove_background_removed.py (CellBender/mitobender fully removed, only a deprecation stub remains); add gzipped-barcode end-to-end tests for bcall and tenx. - CI: add .github/workflows/ci.yml (unit matrix + snakemake/R end-to-end) and rewrite the stale .travis.yml to actually run the suite. - README: replace dead Travis badge with the Actions badge and use the dynamic shields.io PyPI version badge. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 96 +++++++++++++++++++++++++ .travis.yml | 27 +++++-- README.md | 5 +- mgatk/__init__.py | 6 +- mgatk/cli.py | 3 +- mgatk/deletioncalling/clidel.py | 3 +- mgatk/deletioncalling/clifind.py | 3 +- mgatk/mgatkHelp.py | 2 +- tests/test_cli.py | 50 +++++++++++++ tests/test_remove_background_removed.py | 66 +++++++++++++++++ tests/test_version.py | 44 ++++++++++++ 11 files changed, 288 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 tests/test_remove_background_removed.py create mode 100644 tests/test_version.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..fc1ab51 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,96 @@ +name: CI + +on: + push: + branches: [master, main] + pull_request: + branches: [master, main] + +jobs: + # --------------------------------------------------------------------------- + # Fast job: install the package on a Python matrix, confirm every console + # script imports and reports its version, and run the pure-Python unit tests. + # The R/snakemake-dependent end-to-end tests skip themselves here (see the + # `requires_*` markers in tests/conftest.py). + # --------------------------------------------------------------------------- + unit: + name: Unit tests (Python ${{ matrix.python-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.12"] + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: pip + cache-dependency-path: setup.py + + - name: Install mgatk + test dependencies + run: | + python -m pip install --upgrade pip + pip install -e ".[test]" + + - name: Console scripts resolve and report a version + run: | + mgatk --version + mgatk-del --version + mgatk-del-find --version + + - name: Run test suite (end-to-end tests auto-skip without R) + run: pytest -v + + # --------------------------------------------------------------------------- + # Full job: adds R + Bioconductor and runs the complete suite, including the + # snakemake/R end-to-end pipeline tests against the bundled sample data. + # snakemake is installed into the same environment as the package, so the + # subprocess it launches can import mgatk.processing. + # --------------------------------------------------------------------------- + end-to-end: + name: End-to-end (snakemake + R/Bioconductor) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: pip + cache-dependency-path: setup.py + + - name: Set up R + uses: r-lib/actions/setup-r@v2 + with: + r-version: release + use-public-rspm: true + + - name: Cache R packages + uses: actions/cache@v4 + with: + path: ${{ env.R_LIBS_USER }} + key: ${{ runner.os }}-rpkgs-${{ hashFiles('.github/workflows/ci.yml') }} + restore-keys: ${{ runner.os }}-rpkgs- + + - name: Install R / Bioconductor packages + run: | + Rscript -e 'install.packages(c("data.table", "Matrix", "dplyr"), repos = "https://cloud.r-project.org")' + Rscript -e 'if (!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager", repos = "https://cloud.r-project.org"); BiocManager::install(c("SummarizedExperiment", "GenomicRanges"), update = FALSE, ask = FALSE)' + + - name: Install mgatk + test dependencies + run: | + python -m pip install --upgrade pip + pip install -e ".[test]" + + - name: Confirm snakemake and R are visible to mgatk + run: | + mgatk --version + snakemake --version + Rscript -e 'stopifnot(all(sapply(c("data.table", "SummarizedExperiment", "GenomicRanges", "Matrix", "dplyr"), requireNamespace, quietly = TRUE)))' + + - name: Run full test suite + run: pytest -v diff --git a/.travis.yml b/.travis.yml index eba990d..5d89361 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,15 +1,28 @@ language: python -sudo: required -dist: trusty +dist: jammy python: - - "nightly" + - "3.11" + - "3.12" +# R + Bioconductor are needed for the end-to-end pipeline tests. snakemake is a +# pip dependency of mgatk, so it lands in the same environment as the package +# and its subprocess can import mgatk.processing. before_install: - - sudo apt-get install default-jre r-base-dev -y + - sudo apt-get update + - sudo apt-get install -y r-base install: - - python setup.py install + - python -m pip install --upgrade pip + - pip install -e ".[test]" + - Rscript -e 'install.packages(c("data.table", "Matrix", "dplyr"), repos = "https://cloud.r-project.org")' + - Rscript -e 'if (!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager", repos = "https://cloud.r-project.org"); BiocManager::install(c("SummarizedExperiment", "GenomicRanges"), update = FALSE, ask = FALSE)' -script: - - cd tests && echo $PATH +script: + # Console scripts must import and report a version (regression guard for the + # module-level __version__ that previously crashed every entry point). + - mgatk --version + - mgatk-del --version + - mgatk-del-find --version + # Full suite, including the snakemake/R end-to-end tests on the bundled data. + - pytest -v diff --git a/README.md b/README.md index 0907b21..5bc5ceb 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,9 @@

-[![Build Status](https://app.travis-ci.com/caleblareau/mgatk.svg?branch=master)](https://app.travis-ci.com/caleblareau/mgatk) -[![PyPI version](https://badge.fury.io/py/mgatk.svg)](https://pypi.python.org/pypi/mgatk) +[![CI](https://github.com/caleblareau/mgatk/actions/workflows/ci.yml/badge.svg)](https://github.com/caleblareau/mgatk/actions/workflows/ci.yml) +[![PyPI version](https://img.shields.io/pypi/v/mgatk.svg)](https://pypi.python.org/pypi/mgatk) +[![Python versions](https://img.shields.io/pypi/pyversions/mgatk.svg)](https://pypi.python.org/pypi/mgatk) [![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) [![Downloads](https://pepy.tech/badge/mgatk/month)](https://pepy.tech/project/mgatk) diff --git a/mgatk/__init__.py b/mgatk/__init__.py index 3699b0d..edf94b4 100644 --- a/mgatk/__init__.py +++ b/mgatk/__init__.py @@ -1,2 +1,6 @@ +from importlib.metadata import PackageNotFoundError, version -__version__ = '0.7.1' +try: + __version__ = version("mgatk") +except PackageNotFoundError: # running from a source tree without an install + __version__ = "0.8.0" diff --git a/mgatk/cli.py b/mgatk/cli.py index 30ea533..4297c01 100644 --- a/mgatk/cli.py +++ b/mgatk/cli.py @@ -11,7 +11,7 @@ import math import glob -from importlib.metadata import version +from mgatk import __version__ from .mgatkHelp import * from .processing.barcodes import quantify_barcodes, split_barcoded_bam from ruamel.yaml import YAML @@ -82,7 +82,6 @@ def main(mode, input, output, name, mito_genome, ncores, script_dir = os.path.dirname(os.path.realpath(__file__)) cwd = os.getcwd() - __version__ = version('mgatk') click.echo(gettime() + "mgatk v%s" % __version__) if max_javamem is not None: diff --git a/mgatk/deletioncalling/clidel.py b/mgatk/deletioncalling/clidel.py index ae70efe..af8624a 100644 --- a/mgatk/deletioncalling/clidel.py +++ b/mgatk/deletioncalling/clidel.py @@ -10,7 +10,7 @@ import pysam import glob -from importlib.metadata import version +from mgatk import __version__ from mgatk.mgatkHelp import * from ruamel.yaml import YAML from ruamel.yaml.scalarstring import SingleQuotedScalarString as sqs @@ -50,7 +50,6 @@ def main(input, output, name, mito_chromosome, ncores, script_dir = os.path.dirname(os.path.realpath(__file__)) cwd = os.getcwd() - __version__ = version('mgatk') click.echo(gettime() + "mgatk-del v%s" % __version__) # Determine cores diff --git a/mgatk/deletioncalling/clifind.py b/mgatk/deletioncalling/clifind.py index 093e1b5..b4255e2 100644 --- a/mgatk/deletioncalling/clifind.py +++ b/mgatk/deletioncalling/clifind.py @@ -12,7 +12,7 @@ import re import csv -from importlib.metadata import version +from mgatk import __version__ @click.command() @@ -40,7 +40,6 @@ def gettime(): script_dir = os.path.dirname(os.path.realpath(__file__)) cwd = os.getcwd() - __version__ = version('mgatk') click.echo(gettime() + "mgatk-del-find v%s" % __version__) R_plot_script = script_dir + "/bulk_del/plot_deletion_breaks_bulk.R" diff --git a/mgatk/mgatkHelp.py b/mgatk/mgatkHelp.py index 8cfe326..914a8a8 100644 --- a/mgatk/mgatkHelp.py +++ b/mgatk/mgatkHelp.py @@ -180,7 +180,7 @@ def _open_barcode_file(fname): return open(fname) def file_len(fname): - with open(fname) as f: + with _open_barcode_file(fname) as f: return sum(1 for _ in f) def split_barcodes_file(barcode_file, nsamples, output): diff --git a/tests/test_cli.py b/tests/test_cli.py index 70e4c99..8f03aa8 100755 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -137,3 +137,53 @@ def test_tenx_end_to_end_on_known_barcodes(runner, tmp_path, barcode_dir): "n_cells_conf_detected", "n_cells_over_5", "n_cells_over_10", "n_cells_over_20", "n_cells_over_95", "max_heteroplasmy", "strand_correlation", "mean_coverage", ] + + +# --------------------------------------------------------------------------- +# Gzipped barcode lists must be accepted in `bcall` and `tenx` mode (issue #106). +# --------------------------------------------------------------------------- + +def _gzip_barcodes(src_txt, dst_gz): + with open(src_txt) as fh: + contents = fh.read() + with gzip.open(dst_gz, "wt") as fh: + fh.write(contents) + return dst_gz + + +@requires_snakemake +@requires_mgatk_r_packages +def test_bcall_end_to_end_on_gzipped_barcodes(runner, tmp_path, barcode_dir): + barcodes_gz = _gzip_barcodes( + barcode_dir / "test_barcodes.txt", tmp_path / "barcodes.txt.gz" + ) + out_dir = tmp_path / "out" + result = runner.invoke( + cli.main, + [ + "bcall", "-i", str(barcode_dir / "test_barcode.bam"), "-n", "bc1", + "-o", str(out_dir), "-bt", "CB", "-b", str(barcodes_gz), + "-z", "--snake-stdout", + ], + ) + assert result.exit_code == 0, result.output + assert (out_dir / "final" / "bc1.rds").exists() + + +@requires_snakemake +@requires_mgatk_r_packages +def test_tenx_end_to_end_on_gzipped_barcodes(runner, tmp_path, barcode_dir): + barcodes_gz = _gzip_barcodes( + barcode_dir / "test_barcodes.txt", tmp_path / "barcodes.txt.gz" + ) + out_dir = tmp_path / "out" + result = runner.invoke( + cli.main, + [ + "tenx", "-i", str(barcode_dir / "test_barcode.bam"), "-n", "bc1", + "-o", str(out_dir), "-bt", "CB", "-b", str(barcodes_gz), + "-c", "2", "--snake-stdout", + ], + ) + assert result.exit_code == 0, result.output + assert (out_dir / "final" / "bc1.rds").exists() diff --git a/tests/test_remove_background_removed.py b/tests/test_remove_background_removed.py new file mode 100644 index 0000000..38e82d7 --- /dev/null +++ b/tests/test_remove_background_removed.py @@ -0,0 +1,66 @@ +""" +Validates that the CellBender / mitobender "remove-background" functionality has +been removed from mgatk and only survives as a graceful, no-op deprecation stub. + +The heavy background-removal implementation used to live in-tree; it now points +users at the external mitobender project. These tests guard against the +implementation (or its imports) sneaking back in. +""" +import importlib.util + +import pytest +from click.testing import CliRunner + +from mgatk import cli + + +@pytest.fixture +def runner(): + return CliRunner() + + +def test_remove_background_is_a_deprecation_stub(runner, tmp_path): + result = runner.invoke(cli.main, ["remove-background", "-i", "."]) + assert result.exit_code == 0, result.output + assert "deprecated" in result.output + assert "mitobender" in result.output + + +def test_remove_background_does_no_processing(runner, tmp_path): + out_dir = tmp_path / "out" + result = runner.invoke( + cli.main, ["remove-background", "-i", ".", "-o", str(out_dir)] + ) + assert result.exit_code == 0, result.output + # The stub must not spin up any pipeline output directories. + assert not out_dir.exists() + + +@pytest.mark.parametrize( + "module_name", + [ + "mgatk.cellbender", + "mgatk.mitobender", + "mgatk.processing.cellbender", + "mgatk.processing.remove_background", + "mgatk.remove_background", + ], +) +def test_no_background_removal_implementation_modules(module_name): + assert importlib.util.find_spec(module_name) is None + + +def test_deprecated_cellbender_flags_are_hidden_noops(runner): + """The old --ncells_fg/--ncells_bg CellBender knobs remain accepted (so old + scripts don't break) but are hidden and do nothing.""" + help_result = runner.invoke(cli.main, ["--help"]) + assert help_result.exit_code == 0 + assert "ncells_fg" not in help_result.output + assert "ncells_bg" not in help_result.output + assert "CellBender" not in help_result.output + + # Still parsed without error when explicitly supplied. + result = runner.invoke( + cli.main, ["remove-background", "-i", ".", "-nfg", "5", "-nbg", "9"] + ) + assert result.exit_code == 0, result.output diff --git a/tests/test_version.py b/tests/test_version.py new file mode 100644 index 0000000..2dd9a90 --- /dev/null +++ b/tests/test_version.py @@ -0,0 +1,44 @@ +""" +Regression tests for CLI version handling. + +PR #107 moved version resolution off of the removed ``pkg_resources`` API. +A later merge left ``@click.version_option(version=__version__, ...)`` at module +import time while the module-level ``__version__`` binding was dropped, so every +console script crashed with ``NameError`` on import. These tests lock in that the +version is resolvable at import time and surfaced through ``--version``. +""" +from importlib.metadata import version + +import mgatk +from mgatk import cli +from mgatk.deletioncalling import clidel, clifind + +from click.testing import CliRunner + + +EXPECTED_VERSION = version("mgatk") + + +def test_package_version_matches_installed_metadata(): + assert mgatk.__version__ == EXPECTED_VERSION + + +def test_cli_version_option(): + result = CliRunner().invoke(cli.main, ["--version"]) + assert result.exit_code == 0, result.output + assert "mgatk" in result.output + assert EXPECTED_VERSION in result.output + + +def test_del_version_option(): + result = CliRunner().invoke(clidel.main, ["--version"]) + assert result.exit_code == 0, result.output + assert "mgatk-del" in result.output + assert EXPECTED_VERSION in result.output + + +def test_del_find_version_option(): + result = CliRunner().invoke(clifind.main, ["--version"]) + assert result.exit_code == 0, result.output + assert "mgatk-del-find" in result.output + assert EXPECTED_VERSION in result.output From 13745b6c2b089104d0d43999c6040c78edbf3e7a Mon Sep 17 00:00:00 2001 From: caleblareau Date: Wed, 8 Jul 2026 12:55:42 -0400 Subject: [PATCH 2/2] CI: gate del-find plot test on R packages; install ggplot2/ggrepel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_del_find_produces_clip_and_sa_tables invokes mgatk-del-find, which runs its bulk-deletion plot via subprocess.run([...], check=True) — so the command (and the test) hard-fails wherever Rscript or the plotting packages are absent, e.g. the unit CI job with no R. Guard it with a requires_del_plot_r_packages marker (data.table/ggrepel/ggplot2/dplyr) so it skips cleanly there, and add ggplot2/ggrepel to the end-to-end R install so it is still exercised. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 2 +- .travis.yml | 2 +- tests/conftest.py | 7 +++++++ tests/test_deletioncalling.py | 7 ++++++- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc1ab51..2483362 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,7 +78,7 @@ jobs: - name: Install R / Bioconductor packages run: | - Rscript -e 'install.packages(c("data.table", "Matrix", "dplyr"), repos = "https://cloud.r-project.org")' + Rscript -e 'install.packages(c("data.table", "Matrix", "dplyr", "ggplot2", "ggrepel"), repos = "https://cloud.r-project.org")' Rscript -e 'if (!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager", repos = "https://cloud.r-project.org"); BiocManager::install(c("SummarizedExperiment", "GenomicRanges"), update = FALSE, ask = FALSE)' - name: Install mgatk + test dependencies diff --git a/.travis.yml b/.travis.yml index 5d89361..3626b46 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ before_install: install: - python -m pip install --upgrade pip - pip install -e ".[test]" - - Rscript -e 'install.packages(c("data.table", "Matrix", "dplyr"), repos = "https://cloud.r-project.org")' + - Rscript -e 'install.packages(c("data.table", "Matrix", "dplyr", "ggplot2", "ggrepel"), repos = "https://cloud.r-project.org")' - Rscript -e 'if (!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager", repos = "https://cloud.r-project.org"); BiocManager::install(c("SummarizedExperiment", "GenomicRanges"), update = FALSE, ask = FALSE)' script: diff --git a/tests/conftest.py b/tests/conftest.py index 7b3c27f..dcbb41a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -38,6 +38,13 @@ def _r_packages_available(packages): not _r_packages_available(["dplyr"]), reason="R with dplyr is required" ) +# mgatk-del-find shells out to Rscript for its bulk-deletion plot (with +# check=True), so the whole command fails if these plotting packages are absent. +requires_del_plot_r_packages = pytest.mark.skipif( + not _r_packages_available(["data.table", "ggrepel", "ggplot2", "dplyr"]), + reason="R with data.table/ggrepel/ggplot2/dplyr is required for del-find plotting", +) + @pytest.fixture(scope="session") def repo_root(): diff --git a/tests/test_deletioncalling.py b/tests/test_deletioncalling.py index 491ab33..0063d65 100644 --- a/tests/test_deletioncalling.py +++ b/tests/test_deletioncalling.py @@ -4,7 +4,11 @@ from click.testing import CliRunner from mgatk.deletioncalling import clidel, clifind -from conftest import requires_dplyr, requires_snakemake +from conftest import ( + requires_del_plot_r_packages, + requires_dplyr, + requires_snakemake, +) @pytest.fixture @@ -12,6 +16,7 @@ def runner(): return CliRunner() +@requires_del_plot_r_packages def test_del_find_produces_clip_and_sa_tables(runner, tmp_path, pearsonbam_dir): bam = pearsonbam_dir / "CACCACTAGGAGGCGA-1.qc.bam" out_prefix = tmp_path / "out"