[featurizer] added alignment to reference for selection based features. #1184
Conversation
|
@gph82 can you take a look please? |
|
|
||
| def transform(self, traj): | ||
| aligned = traj.superpose(reference=self.ref, atom_indices=self.atom_indices, | ||
| ref_atom_indices=self.ref_atom_indices) |
There was a problem hiding this comment.
I would add a warning in the docs that using the alignment modifies the input md.Trajectory object (because traj.superpose returns self).
|
Otherwise, LGTM |
|
I've thought about adding a 'in_place' option, but actually the
Trajectory object is never seen by the user, so it should be safe to
modify it in place.
|
|
except when using the transform method on its own! |
|
On 11/14/2017 04:39 PM, Guillermo Pérez-Hernández wrote:
except when using the transform method on its own!
Are you doing that? It is not the usual case.
|
|
I am doing that very often! |
|
ok, added this just for you :) but it is not bound to the featurizer api, but lives directly in the feature class. |
|
sure, that is fine. You can make it even just in the doc of the .transform method. But I definitely use feat.transform() very often, quite a lot indeed, and it is actually very useful. I don't think is is unusual |
|
oh, I just meant add to the doc that there's an inplace transformation, not the option.... |
|
I know @cwehmeyer will appreciate this, I wrote a workaround using add_custom_func |
Codecov Report
@@ Coverage Diff @@
## devel #1184 +/- ##
==========================================
- Coverage 90.86% 90.78% -0.09%
==========================================
Files 203 202 -1
Lines 21168 20949 -219
==========================================
- Hits 19235 19018 -217
+ Misses 1933 1931 -2
Continue to review full report at Codecov.
|
2 participants
No description provided.