Conversation
There was a problem hiding this comment.
Pull request overview
Updates the project’s declared Python support and CI test matrix to include Python 3.13, alongside related packaging metadata and dependency adjustments.
Changes:
- Raise minimum supported Python from 3.10 to 3.11 and add the Python 3.13 classifier.
- Add Python 3.13 to the GitHub Actions test matrix.
- Add several new NLP/ML runtime dependencies (torch/transformers/sentence-transformers/tokenizers).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pyproject.toml |
Updates supported Python range/classifiers and adds new dependencies. |
CHANGELOG.md |
Adds new version headings/placeholders for 0.6.x and 0.5.3. |
.github/workflows/test.yml |
Expands CI matrix to run tests on Python 3.13 (and drops 3.10). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| description = "Consensus prediction of cell type labels with popV" | ||
| readme = "README.md" | ||
| requires-python = ">=3.10" | ||
| requires-python = ">=3.11" |
There was a problem hiding this comment.
Raising the minimum supported Python to >=3.11 makes the existing runtime version warning in popv/init.py (it currently checks sys.version_info[:2] != (3, 10) while mentioning Python 3.11) trigger on every supported Python version. Please update/remove that warning (e.g., check for != (3, 11) or format the f-string correctly) so users on supported versions don’t always see a misleading warning.
| "torch>=2.4", | ||
| "transformers>=4.35.0", | ||
| "sentence-transformers>=2.3.1", | ||
| "tokenizers>=0.14.0", |
There was a problem hiding this comment.
These newly-added heavyweight NLP deps (torch/transformers/sentence-transformers/tokenizers) significantly increase install size/time and may not be needed for typical popV usage (they appear to be required mainly for create_ontology_resources -> SentenceTransformer). Consider moving them into an optional extra (e.g., nlp) and raising a clear ImportError with install instructions when the feature is used without the extra, instead of making them unconditional core dependencies.
| description = "Consensus prediction of cell type labels with popV" | ||
| readme = "README.md" | ||
| requires-python = ">=3.10" | ||
| requires-python = ">=3.11" |
There was a problem hiding this comment.
PR title suggests this is only adding Python 3.13 to the test matrix, but this diff also changes the package’s supported Python range (drops 3.10 by setting requires-python to >=3.11) and adds new runtime dependencies. Please update the PR title/description to reflect the full scope, or split the support/dependency changes into a separate PR for clearer review.
No description provided.