Refactor alignment to allow passing through a list of ligands (for SepTop)#110
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 98.05% 98.26% +0.21%
==========================================
Files 9 9
Lines 462 461 -1
==========================================
Hits 453 453
+ Misses 9 8 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| u.trajectory.close() | ||
|
|
||
|
|
||
| def test_chain_radius_of_gyration_stable(universe_single_state): |
There was a problem hiding this comment.
I moved these tests from test_transformations.py since they are testing these utils apply_transformations functions.
IAlibay
left a comment
IAlibay
left a comment
There was a problem hiding this comment.
Couple of small things, otherwise it looks good to me. I'll approve early.
| """ | ||
| has_protein = protein is not None and protein.n_atoms > 0 | ||
| has_ligand = ligand is not None and ligand.n_atoms > 0 | ||
| if protein is None or protein.n_atoms == 0: |
There was a problem hiding this comment.
You can do "not protein" here instead of checking for n_atoms.
| """ | ||
| Build transformations for systems containing a protein and optionally | ||
| a ligand. | ||
| ligands = [lig for lig in (ligands or []) if lig.n_atoms > 0] |
There was a problem hiding this comment.
as above, you can rely on bool(lig) == False when the atomgroup is empty
| @@ -22,99 +22,84 @@ def apply_alignment_transformations( | |||
| The Universe the transformations are applied to. Modified in-place. | |||
| protein: mda.AtomGroup | None | |||
There was a problem hiding this comment.
need to update docstring for non-optional protein parameter
| ], | ||
| ) | ||
| def test_separate_ligands_fixes_pbc_spike( | ||
| self, septop_complex_data, state_idx, ligand_key, expected_spike |
There was a problem hiding this comment.
The expected_spike parameter isn't used for anything it seems.
| # Combined approach should produce a spike | ||
| u_combined = universe_utils.create_universe_single_state(d["pdb"], ds, state=state_idx) | ||
| prot = u_combined.select_atoms("protein and name CA") | ||
| lig_A = u_combined.atoms[np.array(d["ligand_A_indices"])] |
There was a problem hiding this comment.
You use np.array here and not below. I think you can just use the bare list in both?
2 participants
I was working on adding the structural analysis to the SepTop protocol and I realized that when I pass the ligands to the alignement as a combined atomgroup, but one ligand is in a different image than the other ligand, this can lead to spiked in the RMSD. I changed the alignment, now allowing passing in a list of AtomGroups for the ligands.