feat: better typing, dropped py 3.10 support #417
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request drops Python 3.10 support and improves type annotations by replacing string-based forward references with the Self type hint introduced in Python 3.11.
Changes:
- Updated minimum Python version requirement from 3.10 to 3.11 across project configuration and CI/CD workflows
- Replaced forward reference return type annotations (e.g.,
"Protein","Ligand") withSelftype hint in class methods - Refactored
@beartypedecorator usage by moving it to class level forLigandandLigandSetclasses - Enhanced type hints for file path parameters to accept both
strandPathtypes - Improved
LigandSet.from_dir()implementation with better error handling and fixed a bug where CSV ligands were not properly added to the result
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Updated minimum Python version requirement from 3.10 to 3.11 |
| docs/install.md | Removed Python 3.10 from the platform compatibility table |
| .github/workflows/test_docs.yaml | Updated Python version from 3.10 to 3.11 |
| .github/workflows/pypi.yml | Updated Python setup from 3.10 to 3.11 |
| .github/workflows/main.yml | Removed 3.10 from test matrix |
| .github/workflows/docs.yml | Updated Python version from 3.10 to 3.11 |
| src/drug_discovery/structures/protein.py | Added Self import and updated return type annotations; removed @beartype decorator from from_base64 method |
| src/drug_discovery/structures/pocket.py | Added Self import and updated return type annotations for class methods |
| src/drug_discovery/structures/ligand.py | Added Self import, updated return types, added class-level @beartype, improved file path type hints, enhanced from_dir() implementation, and fixed bug in CSV file processing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return [ligand.mol for ligand in self.ligands] | ||
|
|
||
| def mcs(self) -> str: | ||
| def mcs(self) -> Chem.Mol: |
There was a problem hiding this comment.
The return type annotation was changed from str to Chem.Mol. This appears to be a typing improvement, but the method's docstring (not shown in this diff but exists in the codebase) may need to be updated to reflect this change if it previously indicated a string return type.
| def from_base64( | ||
| cls, | ||
| base64_string: str, | ||
| name: str = "", | ||
| **kwargs: Any, | ||
| ) -> "Protein": | ||
| ) -> Self: |
There was a problem hiding this comment.
The @beartype decorator was removed from this method, but unlike the Ligand and LigandSet classes, the Protein class does not have @beartype applied at the class level. This means this method (and potentially others in this class) will no longer have runtime type checking, which appears to be an unintended inconsistency with the changes made to ligand.py where @beartype was moved to the class level.
| file_path: str | Path, | ||
| *, | ||
| sanitize: bool = True, | ||
| remove_hydrogens: bool = False, | ||
| ) -> "Ligand": | ||
| ) -> Self: |
There was a problem hiding this comment.
The parameter type annotation was changed to accept both str and Path (line 273), but the docstring on line 282 (not shown in this diff but part of the method) likely still states file_path (str). Consider updating the docstring to reflect the new type: file_path (str | Path).
changes