Boltzina affinity#54
Conversation
…oved logging messages from boltzina/reactifptm
…with ipTM columns
| def resolve_cache_path(cache_dir: Optional[Path] = None) -> Path: | ||
| """Return the Boltz cache directory (holds ``ccd.pkl`` and ``mols/``). | ||
|
|
||
| ``boltz.main.get_cache_path`` has moved/disappeared across Boltz versions, |
There was a problem hiding this comment.
I'd remove working notes from the doc strings. It has the ability to snowball with too may changes and changes should be tracked in git
| def ensure_boltz_cache(cache_dir: Path) -> None: | ||
| """Make sure the Boltz molecule/CCD cache is downloaded. | ||
|
|
||
| The ``boltz predict`` CLI downloads ``ccd.pkl`` / ``mols/`` / weights into |
There was a problem hiding this comment.
Same here. this much information isn't need in the docstring
| if mol_dir.exists() and any(mol_dir.iterdir()): | ||
| return | ||
|
|
||
| from boltz import main as bm |
There was a problem hiding this comment.
Is there a reason this is here and not at the top of the file?
| matching how Boltzina prepares ligand mols. | ||
| """ | ||
| from rdkit import Chem | ||
| from rdkit.Chem import AllChem |
There was a problem hiding this comment.
Same with these, why not at the top of the file. I'm surprised the pre-commit doesn't catch these
|
|
||
| Boltz expects one ``{record_id}.pkl`` per record, each a | ||
| ``{ligand_name: rdkit_mol}`` dict, located in the processed ``mols`` dir. | ||
| """ |
There was a problem hiding this comment.
I don't mind the helper functions to not have the right style doc strings but this should. Also a type hint for mol
| Boltz expects one ``{record_id}.pkl`` per record, each a | ||
| ``{ligand_name: rdkit_mol}`` dict, located in the processed ``mols`` dir. | ||
| """ | ||
| from rdkit import Chem |
There was a problem hiding this comment.
Imported multiple times across multiple functions
There was a problem hiding this comment.
The doc strings need to be revisited for consistency, the imports should be moved to the top of the file and some arguments didn't have type hints
| logger.warning("No affinity results were produced at %s", results_csv) | ||
| return {} | ||
|
|
||
| import pandas as pd |
There was a problem hiding this comment.
Move to the top of the file
hllelli2
left a comment
hllelli2
left a comment
There was a problem hiding this comment.
Overall, the code seems fine but AI does code very well. If using Claude, I'd recommend setting up some skills which help ensure consistency in coding style such as " /code_audit ". The big things to address are the docstrings, The style has gone from google to whatever is there now on new functions. This should be refactored for consistency. There's also may imports which are in functions ad sometimes in the middle of functions. These should be moved to the top of files. Another thing I noticed was there is a lot of new additions regarding boltzina but I dont see any new tests. Make sure these are written so that functionality can be maintained and assessed.
| # --------------------------------------------------------------------------- # | ||
| # Ligand -> RDKit mol | ||
| # --------------------------------------------------------------------------- # | ||
| def write_ligand_pdb( |
| mol, | ||
| record_ids: List[str], | ||
| base_extra_mols_dir: Path, | ||
| ligand_name: str = "LIG", |
| record_ids: List[str], | ||
| out_manifest: Path, | ||
| ) -> Path: | ||
| """Clone the base record into one record per ``record_ids`` entry.""" |
| # Results | ||
| # --------------------------------------------------------------------------- # | ||
| def extract_affinity_results( | ||
| predictions_dir: Path, |
hllelli2
left a comment
hllelli2
left a comment
There was a problem hiding this comment.
Commenting until boltzina tests add ons
2 participants
Uh oh!
There was an error while loading. Please reload this page.