fixed make_whole only working on 0 base AtomGroups#1987
fixed make_whole only working on 0 base AtomGroups#1987richardjgowers merged 3 commits intodevelopfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1987 +/- ##
===========================================
- Coverage 88.48% 88.45% -0.03%
===========================================
Files 142 142
Lines 17202 17202
Branches 2635 2635
===========================================
- Hits 15221 15216 -5
- Misses 1385 1389 +4
- Partials 596 597 +1
Continue to review full report at Codecov.
|
|
I tries it and it appears to work. We are missing two tests, though: a test where the AtomGroup that is passed to |
richardjgowers
force-pushed
the
make_whole_fix
branch
from
a3ad3dd to
1ca2bc7
Compare
| # Draw vector from atom to other | ||
| for i in range(3): | ||
| vec[i] = oldpos[other, i] - oldpos[atom, i] | ||
| vec[i] = oldpos[other, i] - newpos[atom, i] |
There was a problem hiding this comment.
because we're using newpos[atom, i] later (and is equivalent to oldpos[atom, i]), maybe this can cause a cache hit
|
@jbarnoud should be good to go |
|
The linter is crying and I'm not sure why, and I can't reproduce it locally. Thanks pylint |
|
It looks like pylint got updated and now detects new things. The fixes should probably go in an other PR. |
jbarnoud
left a comment
jbarnoud
left a comment
There was a problem hiding this comment.
I needed it today, and it works just as expected. Along side @davidercruz's on-the-fly transformation it is pure magic.
I'll see how long it takes to fix the pylint error before I merge, though.
| - env: NAME="Lint" | ||
| PYLINTRC="${TRAVIS_BUILD_DIR}/package/.pylintrc" | ||
| MAIN_CMD="pylint package/MDAnalysis && pylint testsuite/MDAnalysisTests" | ||
| MAIN_CMD="pylint --rcfile=$PYLINTRC package/MDAnalysis && pylint --rcfile=$PYLINTRC testsuite/MDAnalysisTests" |
There was a problem hiding this comment.
this isn't changing anything. It should already pick up the right RC file
3 participants
@jbarnoud the bug you found was actually worse than initially thought.
bonds.to_indicesgave the global index of the atoms involved, whereas indexing.positionsuses local indices (ie the index within the AtomGroup)