update pacbio samplesheet notebook, tests, and test samplesheets#365
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the PacBio sample sheet builder notebook, its tests, and associated test data to better handle different combinations of amplification and absolute quantification workflows. The changes improve test naming conventions, fix absquant detection logic, and expand test coverage with more comprehensive test cases.
Changes:
- Renamed and restructured test methods with clearer naming (case1-5 with descriptive conditions)
- Fixed absquant detection logic in the notebook to check for actual values instead of just column existence
- Added/updated test data files covering five distinct workflow cases: metagenomic without amplification/absquant, absquant without amplification, metagenomic with amplification, and absquant with amplification (syndna before/after)
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| notebooks/tests/test_pacbio_sample_sheet_builder.py | Renamed test methods with clearer case-based naming and added new test case5 for syndna-after workflow |
| notebooks/pacbio_sample_sheet_builder.ipynb | Fixed absquant detection logic to properly check if mass_syndna_input_ng column has actual values |
| notebooks/test_data/processing_docs/*.csv | Added/updated processing doc test data for five distinct workflow cases |
| notebooks/test_output/SampleSheets/*.csv | Added/updated expected sample sheet outputs for all test cases, including barcode corrections in case4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'pacbio_v11_absquant_sample_sheet_case5_with_amp_' | ||
| 'with_absquant_syndna_after.csv'), |
There was a problem hiding this comment.
My only comment with is out of the scope of this PR is that splitting this strings is not convenient when you want to look for this strings in the code - I was trying to verify that all the csv are being tested. What about replacing flake8 for ruff, it's pretty easy and I can do it - when we chat let's discuss this.
|
Thank you @AmandaBirmingham, looks good! |
No description provided.