Skip to content

Fixed wrong atom sorting in _split_by_compound_indices#3353

Merged
richardjgowers merged 1 commit intodevelopfrom
issue-3353-unwrapsort
Jun 28, 2021
Merged

Fixed wrong atom sorting in _split_by_compound_indices#3353
richardjgowers merged 1 commit intodevelopfrom
issue-3353-unwrapsort

Conversation

@mnmelo
Copy link
Member

@mnmelo mnmelo commented Jun 10, 2021

Closes #3353

Changes made in this Pull Request:
_split_by_compound_indices was not properly propagating the sorting when sorting was needed.

Tested initially on an example LAMMPS structure provided by @hejamu, but found that 1hvr.pdb (MDAnalysisTests.datafiles.CONECT) also elicited the same problem when unwrapping. The implemented test uses the latter, but the fix does work for the LAMMPS case too.

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@mnmelo mnmelo requested a review from richardjgowers June 10, 2021 12:12
@IAlibay IAlibay added this to the 2.0 milestone Jun 10, 2021
@mnmelo
Copy link
Member Author

mnmelo commented Jun 10, 2021

(I pinged you, @richardjgowers, because this is essentially a fix over #3005, which you reviewed).

mnmelo added a commit that referenced this pull request Jun 10, 2021
@mnmelo mnmelo force-pushed the issue-3353-unwrapsort branch from 6496e05 to 4c3e92d Compare June 10, 2021 12:19
@codecov
Copy link

codecov bot commented Jun 10, 2021

Codecov Report

Merging #3353 (68c2509) into develop (a6d4ffc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3353   +/-   ##
========================================
  Coverage    93.62%   93.62%           
========================================
  Files          176      176           
  Lines        22837    22838    +1     
  Branches      3225     3225           
========================================
+ Hits         21380    21381    +1     
  Misses        1406     1406           
  Partials        51       51           
Impacted Files Coverage Δ
package/MDAnalysis/core/groups.py 98.48% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6d4ffc...68c2509. Read the comment docs.

@mnmelo mnmelo force-pushed the issue-3353-unwrapsort branch from 4c3e92d to 68c2509 Compare June 10, 2021 17:27
@richardjgowers richardjgowers merged commit 64840d5 into develop Jun 28, 2021
@richardjgowers richardjgowers deleted the issue-3353-unwrapsort branch June 28, 2021 17:25
@IAlibay IAlibay added the defect label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants