-
Notifications
You must be signed in to change notification settings - Fork 11
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 #122 #123
Fix issue #122 #123
Conversation
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.
That is a surprising API - I would have expected compare to return a bool too.
@chryswoods Do you mind back port this into the main as well, please? Thank you. |
Yes - although I will delegate ;-) @lohedges - do you have time to do this tomorrow? I'm at a meeting in London all day. |
Yes, no problem. |
Thanks :-) |
This is now backported into |
This PR closes #122 by fixing the use of
QString::compare
when comparing molecular properties. We added a new (as yet unused)is_non_searchable_water
property to allow a user to preserve water topologies, e.g. by excluding them from a search. However, I don't want to keep this property when swapping topology, so don't copy it to the new water template. I was incorrectly usingQString::compare
since I hadn't realised that it returned anint
, not abool
. The return value is zero only when the strings match.This will be tested via BioSimSpace.
devel
into this branch before issuing this pull request (e.g. by runninggit pull origin devel
): [y]Suggested reviewers:
@chryswoods