Conversation
Contributor
kyoung73
commented
Jul 15, 2025
- Updated README.md to improve badge visibility and added GitHub actions and issues badges.
- Bumped version in pyproject.toml to 1.2.4 and updated deprecated fields.
- Enhanced Annotations class to check for 'SeqId' in both index and columns.
- Added new test cases for handling SeqId in annotations lifting.
- CAN-26
- Updated README.md to improve badge visibility and added GitHub actions and issues badges. - Bumped version in pyproject.toml to 1.2.4 and updated authors and dependencies. - Enhanced Annotations class to check for 'SeqId' in both index and columns. - Added new test cases for handling SeqId in annotations lifting. - CAN-26
Contributor
There was a problem hiding this comment.
Pull Request Overview
Enhance project metadata, documentation, and annotations handling to support SeqId in both index and columns, and expand tests accordingly.
- Improve README badges and remove deprecated anchors.
- Migrate
pyproject.tomlto PEP 621-style metadata, bump version to 1.2.4, and update dependencies. - Extend
Annotations.update_adat_column_metaandAnnotations.lift_adatto detectSeqIdin either index or columns, with new tests covering the scenarios.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_annotation_lifting.py | Refactored test helpers, added constants, and new test cases for SeqId in index/columns |
| somadata/annotations.py | Added SeqId presence checks in both index and columns for updating metadata and lifting |
| pyproject.toml | Converted to [project] table format, bumped version, added requires-python and dependencies |
| README.md | Updated badges and cleaned up redundant “return to top” anchors |
Comments suppressed due to low confidence (2)
tests/test_annotation_lifting.py:23
- [nitpick] The parameter
index_colaccepts bothFalseand a column name string, which can be confusing. Consider renaming it toindex_labelorindex_nameto clarify its purpose.
def create_annotations_from_csv(csv_data: str, index_col=False) -> Annotations:
README.md:109
- [nitpick] There are several 'return to top' links scattered throughout this document. Consider consolidating or removing redundant anchors to improve readability.
[return to top](#toptoc)
jrefi-illumina
approved these changes
Jul 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.