Refactor of universe creation and applying transformations#93
Refactor of universe creation and applying transformations#93hannahbaumann wants to merge 11 commits into
Conversation
| from openfe_analysis.utils.multistate import _determine_position_indices | ||
|
|
||
|
|
||
| def _create_universe_single_state(top, trj, state): |
There was a problem hiding this comment.
I think this should go into utils. The reader file should really concentrate on reader things rather than universe creation.
| ligand: Optional[mda.AtomGroup] = None, | ||
| ): | ||
| """ | ||
| Apply a collection of transformations to a Universe. |
There was a problem hiding this comment.
Since MDAnalysis transformations are meant to be quite flexible and additive, it might be good to be more descriptive about this sentence. Maybe be even just "collection of transformations usually required for X, Y, Z analyses" would be fine.
| u.trajectory.add_transformations(*transforms) | ||
|
|
||
|
|
||
| def _apply_transformations_complex(protein, ligand=None): |
There was a problem hiding this comment.
Please do add type hints everywhere.
|
|
||
|
|
||
| def apply_transformations( | ||
| u: mda.Universe, |
There was a problem hiding this comment.
I often make this mistake, so I'm nit-picking but universe might be better here than u.
|
|
||
| def apply_transformations( | ||
| u: mda.Universe, | ||
| protein: Optional[mda.AtomGroup] = None, |
There was a problem hiding this comment.
Given we're post python 3.9, it might be good to go for mda.AtomGroup | None instead of Optional for new type hints.
| from ..transformations import Aligner, ClosestImageShift, NoJump | ||
|
|
||
|
|
||
| def apply_transformations( |
There was a problem hiding this comment.
Very small nit: is apply_tansformations generic as these are the standard alignment transformations, what about apply_alignment_transformations or apply_analysis_transformations?
jthorton
left a comment
jthorton
left a comment
There was a problem hiding this comment.
Thanks @hannahbaumann looks great, nothing to add over @IAlibay so i'll approve early!
3 participants
The
make_Universefunction did two things: Create a Universe for a state and then applying some transformations to it, hard coding selection strings for protein and ligand.Here, I split off the creation of the universe and the applying of transformations into separate steps. This allows passing AtomGroups to the
apply_transformationsfunction. I thought that this might be useful for moving towards a more individualProtocolbased analysis.