-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix issue #270 #271
Fix issue #270 #271
Conversation
Thinking more about this, I had a similar problem when searching for ions during parameterisation. It wasn't possible to do a search for all ions in one go due to the limitation of the search syntax. Instead I just looped over the ions and searched one-by-one. Perhaps I need to update the code to do something similar here, or maybe it could be caught and handled in the sire search grammar. |
Actually, it appears that the issue was that the protein and nucleic acid residue strings weren't being joined correctly. No comma was added between then so the last protein residue name was being combined with the first nucleic acid residue. Depending on the ordering from the set, this could result in removing some of the matching residues from the test molecule, e.g. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good - the change from { }
to [ ]
is very subtle - good spot that this was one of the causes of the inconsistent behaviour. Same with the missing "," in the join.
This PR closes #270 by using a list rather than a set to store the names of protein and nucleic acid residues. This makes the search string for backbone atoms reproducible, hence the tests now repeatedly pass. I'm still not sure why this issue has only cropped up recently, since
_prot_res
has been a set since the code was first added.devel
into this branch before issuing this pull request (e.g. by runninggit pull origin devel
): [y]Suggested reviewers:
@chryswoods