Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SameSelection should have an option for indices #2669

Closed
lilyminium opened this issue Apr 23, 2020 · 1 comment · Fixed by #2673
Closed

SameSelection should have an option for indices #2669

lilyminium opened this issue Apr 23, 2020 · 1 comment · Fixed by #2673
Assignees
Milestone

Comments

@lilyminium
Copy link
Member

lilyminium commented Apr 23, 2020

Expected behavior

byres and same residue as selects the whole residue for each atom in the subsequent selection, by the canonical and unique resindex. Likewise same segment as uses segindex.

Actual behavior

byres and same residue as use the user-supplied, potentially repeated resid values. same segment as uses segid. This is redundant as you can already use same resid as or same segid as for the same behaviour. This may result in unexpected behaviour with altLocs also.

Code to reproduce the behavior

Leads to problems like in this mailing list issue where selecting "byres resname TIP3" returns various amino acids and ions as well.

>>> import MDAnalysis as mda
>>> u = mda.Universe('a.psf', 'a.dcd')
>>> ag = u.select_atoms('byres resname TIP3')
>>> print(set(ag.resnames))
{'GLN', 'LEU', 'TRP', 'THR', 'LYS', 'GLU', 'POPE', 'ASP', 'TYR', 'VAL', 'ARG', 'PHE', 'MET', 'CYS', 'ILE', 'PRO', 'TIP3', 'ALA', 'SOD', 'CLA', 'SER', 'GLY', 'ASN', 'HSD'}

Current version of MDAnalysis

  • Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 0.20.1
  • Which version of Python (python -V)? 3.7.3
  • Which operating system? MacOS
@richardjgowers
Copy link
Member

Oops yeah, that's the whole reason we have ix and friends

@orbeckst orbeckst added this to the 1.0 milestone May 14, 2020
orbeckst pushed a commit that referenced this issue May 15, 2020
* fixes #2669 
* fixes #2672
* fixes "same residue" and "same segment" selections
  * Use resindices of resids for same residue selection
  * Use segindices of segids for same segment selection
  * Use resindices for byres selection
* add failing test
* failing tests with correct number of atoms
* fix same selection
* add failing byres test
* fix byres test
* add comments
* updated changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants