Skip to content

Remove mix-max dead code and --method CLI flag#30

Open
wsnoble wants to merge 3 commits into
scale-phase-4from
remove-dead-code
Open

Remove mix-max dead code and --method CLI flag#30
wsnoble wants to merge 3 commits into
scale-phase-4from
remove-dead-code

Conversation

@wsnoble

@wsnoble wsnoble commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Removes the MixmaxConfidence class (~177 lines) from confidence.py — the mix-max method was previously removed but its class was left behind
  • Removes mixmax(), estimate_pi0(), and calculate_mixmax_qval() from qvalues.py (~211 lines) — these only served mix-max
  • Removes the --method/-m CLI flag from params.py — its only remaining choice was "tdc", making it a no-op
  • Removes the method parameter from PsmDataset.assign_confidence() and dataset.py's internal dispatch dict, replacing method-based dispatch with a direct TdcConfidence(...) call (the backend parameter added in PR29 is preserved)
  • Cleans up unused imports: itertools in utils.py, ClassVar in dataset.py, re in three parsers, read_txt/read_pepxml in msfragger.py
  • Fixes test fixtures to use Path(__file__).parent.parent / "data" so tests pass regardless of the working directory from which pytest is run

Test plan

  • Full test suite passes: pytest tests/ → 88 passed, 2 skipped
  • All method="tdc" kwargs removed from all test files (test_confidence.py, test_edge_cases.py, test_writers.py)
  • No mixmax fixtures or tests remain in test_qvalues.py or test_confidence.py
  • numba import retained in qvalues.py — still used by _fdr2qvalue

🤖 Generated with Claude Code

Eliminates all mix-max confidence estimation code (MixmaxConfidence
class, mixmax/estimate_pi0/calculate_mixmax_qval functions) that was
left behind after the method was removed. Also drops the --method CLI
flag whose only remaining choice was "tdc", removes the method parameter
from assign_confidence(), cleans up unused imports (itertools, ClassVar,
re, read_txt, read_pepxml), and fixes test fixtures to use absolute
paths so tests pass regardless of working directory.

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

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc79ac62-3844-4eb1-b694-0a6a696fbe6d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-dead-code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

wsnoble and others added 2 commits June 30, 2026 17:13
Install duckdb and pyarrow to enable previously-skipped tests.
Remove stray method= kwargs from test_duckdb.py and test_parquet.py,
and replace the now-defunct test_duckdb_backend_invalid_method_raises
with test_duckdb_backend_invalid_backend_raises (tests unknown backend
name). Also fix parse_psms_txt in utils.py to skip the pyarrow engine
when skip_line=True, since pyarrow does not support skiprows and would
misidentify column names in files with a leading comment row (MSAmanda).

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

Copilot AI 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.

Pull request overview

This PR removes the previously-deprecated MixMax confidence/q-value implementation and simplifies the confidence estimation API/CLI to the single remaining method (TDC), while updating tests and fixtures accordingly.

Changes:

  • Deleted MixMax-related code paths (MixmaxConfidence and mixmax q-value helpers) and removed method-based dispatch in PsmDataset.assign_confidence().
  • Removed the no-op --method/-m CLI flag and updated CLI dispatch to match the new API.
  • Updated unit tests/fixtures to remove method="tdc" usage and to use a stable DATA_DIR path independent of the current working directory.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit_tests/test_writers.py Removes method="tdc" from confidence assignment calls in writer tests.
tests/unit_tests/test_qvalues.py Removes MixMax imports and MixMax test coverage; keeps TDC tests.
tests/unit_tests/test_parquet.py Removes method="tdc" from Parquet writer tests.
tests/unit_tests/test_edge_cases.py Removes method="tdc" from edge-case confidence assignment tests.
tests/unit_tests/test_duckdb.py Updates backend validation test to check unknown backend rather than invalid method.
tests/unit_tests/test_confidence.py Removes MixMax confidence tests and method="tdc" usage; keeps TDC tests.
tests/conftest.py Introduces DATA_DIR fixture root to make test data paths robust to pytest CWD.
crema/utils.py Ensures parse_psms_txt doesn’t use the pyarrow engine when skiprows is required.
crema/qvalues.py Removes MixMax q-value estimation functions, leaving TDC and helper _fdr2qvalue.
crema/params.py Removes the --method/-m CLI argument.
crema/dataset.py Removes method dispatch and always instantiates TdcConfidence for pandas backend; preserves backend selection.
crema/crema.py Stops passing args.method into assign_confidence now that the flag/parameter is removed.
crema/confidence.py Removes the MixmaxConfidence class implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants