`utils.sort_atomgroup` should not modify atomgroup
As recently found by @pstaerk, one can get wrong results with the sort_atomgroup
.
Consider the case of one defining a transformation that uses predefined atom indices to act on. The rearranging in sort_atomgroup
can cause unexpected behavior.
For already correctly arranged atom indices, stable sort quarantines that elements are not shuffles within a fragment/molecule/residue. But for topologies with non-aligned fragments/molecules/residues, this is still a problem.
Proposal:
The sort_atomgroup
method is only needed if we try to access fragments/molecules/residues by indices rather than through the MDAnalysis API. This is only the case in Epsilon. The sort_atomgroup
method could be rewritten not to return the sorted atomgroup, but the indexing that would sort the array. Then this indexing can be used to perform the position manipulation needed in Epsilon, which already happens in a copied array anyhow. This way the atomgroup doesn't get changed.