-
Notifications
You must be signed in to change notification settings - Fork 20
Pacbio 20251014 #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Pacbio 20251014 #339
Conversation
Pull Request Test Coverage Report for Build 19077243312Details
💛 - Coveralls |
…s for amplicon notebook
…human investigation
…med whitespace at the end of code lines
… settings to string constants
There was a problem hiding this comment.
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 migrates the test framework from nose to pytest, updates test configuration, and renames test classes for better consistency. The changes include:
- Replacement of
nosewithpytestandpytest-covin testing dependencies - Addition of
pytest.iniconfiguration file with test discovery settings - Renaming of test classes to follow a more descriptive naming pattern
- Addition of new test files for notebook testing with a helper base class
- Standardization of column name from "sample sheet Sample_ID" to "Sample_ID" across test output files
Reviewed Changes
Copilot reviewed 35 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Replaced nose testing framework with pytest; removed test_suite configuration |
| pytest.ini | New pytest configuration file defining test paths, patterns, and options |
| notebooks/tests/test_tellseq_D_variable_volume_pooling.py | Renamed test class from TestTellseqD to TestTellseqDNotebook; renamed test method for clarity |
| notebooks/tests/test_metatranscriptomics_matrix_pipeline_seqcount_norm.py | New test file for metatranscriptomics notebook testing |
| notebooks/tests/test_matrix_tube_pipeline_seqcount_norm.py | New test file for matrix tube pipeline notebook testing |
| notebooks/tests/test_amplicon_pre_prep_file_generator.py | New test file for amplicon pre-prep notebook testing |
| notebooks/tests/notebook_test_helpers.py | New base class providing common test infrastructure for notebook tests |
| notebooks/test_output files | Updated column header from "sample sheet Sample_ID" to "Sample_ID" for consistency |
| notebooks/init.py | Empty init file added (standard Python package marker) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import os | ||
| import re | ||
|
|
||
| SAVE_DIR = "/Users/amandabirmingham/Desktop" |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded personal directory path should be removed. This exposes a username and is not portable across different development environments. Consider using a temporary directory or making this configurable.
| # TODO: turn off before committing | ||
| _SAVE_UNMATCHED_OUTPUTS = True # whether to save unmatched outputs |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug flag _SAVE_UNMATCHED_OUTPUTS is set to True with a TODO comment to turn it off before committing. This should be set to False by default to avoid saving debug output in production test runs.
| self._FILE_PATH_KEY: True | ||
| }, | ||
|
|
||
| # Step 2 Step 5 output - index picklist |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment contains duplicate 'Step 2' which appears to be a typo. Should likely be 'Step 5 output - index picklist' or clarify which step this refers to.
No description provided.