Skip to content

Added some tests for the rust functions#1369

Closed
gerikson wants to merge 7 commits into
deeptools:4.0.0from
gerikson:tests_ge
Closed

Added some tests for the rust functions#1369
gerikson wants to merge 7 commits into
deeptools:4.0.0from
gerikson:tests_ge

Conversation

@gerikson

Copy link
Copy Markdown
Collaborator

Welcome to deepTools GitHub repository! Please check the following regarding
your pull request :

  • Does the PR contain new feature?
  • Does the PR contain bugfix?
  • Does the PR contain documentation changes?
  • Does the PR contain changes to the galaxy wrapper?
  • [ x] Contains pytests

@gerikson gerikson requested a review from adRn-s January 22, 2025 15:01
@SaimMomin12

SaimMomin12 commented Jan 22, 2025

Copy link
Copy Markdown
Collaborator

@gerikson Can you change the target of this PR to 4.0.0 instead of master branch?

@WardDeb WardDeb changed the base branch from master to 4.0.0 January 22, 2025 15:28
@pavanvidem

Copy link
Copy Markdown
Collaborator

@gerikson FYI, @SaimMomin12 and me are creating tests for plotFingerprint, plotPCA and plotProfiler

adRn-s added a commit that referenced this pull request Jul 8, 2026
Salvaged from #1369: keeps only the computeMatrix test, which passes
against the current Rust bindings. The alignmentSieve, bamCompare and
bamCoverage tests from that PR were dropped because their Rust function
signatures have since changed (and alignmentSieve2 is not yet wired up
on 4.0.0).

Co-authored-by: gerikson <galina.erikson@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@adRn-s

adRn-s commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Partially salvaged in #1433, which keeps the test_r_computeMatrix.py test (verified passing against the current Rust bindings). The other three test files were dropped because the code moved on:

  • test_r_alignmentsieve.py: alignmentSieve2 isn't wired up on 4.0.0 yet (CLI entry point is commented out; the Python wrapper passes 15 args while the compiled binding needs 17), and it panics on an empty blacklist path.
  • test_r_bamCompare.py / test_r_bamCoverage.py: the Rust signatures changed (e.g. bamCoverage now requires collapse; bamCompare gained sf1/sf2, split pseudocounts), so the positional calls no longer line up.

These can be revisited once those tools' Rust APIs settle. Closing in favor of #1433 — thanks @gerikson! 🙏

@adRn-s adRn-s closed this Jul 8, 2026
adRn-s added a commit that referenced this pull request Jul 9, 2026
…t/strand filtering (#1434)

Fills coverage gaps identified while triaging the stalled #1369 tests:
- bamCoverage: --normalizeUsing RPKM (RPKM was untested; suite only had
  CPM/RPGC) and --scaleFactor.
- alignmentSieve: --minFragmentLength/--maxFragmentLength and
  --filterRNAstrand (neither was tested anywhere).

Expected values derived by running the current tools; all pass under
`pixi run pytest`.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

4 participants