-
Notifications
You must be signed in to change notification settings - Fork 657
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
Implicit or in selections (Issue #345) #630
Conversation
Allows selections such as: u.select_atoms('name C N') == u.select_atoms('name C or name N') u.select_atoms('name N and resname GLY LEU') == 'name N and (resname GLY or resname LEU)' Defines how to detect keywords in selection strings (`is_keyword`) Issue #347
Would that screw with us trying to formalize the selection syntax? |
Formalise as in move to pyparsing? |
Yes. Pyparsing would require us to write a formal selection grammar if I remember correctly. |
We've technically already got a language, but yeah it would be another feature we'd have to implement if we moved |
I do wonder if pyparsing is necessary at this point? @richardjgowers has done a lot of work fixing up the selection machinery, and I wonder what we would specifically gain from moving to pyparsing? Also, what would we have to twist pyparsing's arm to do? |
A lot of the work I did wouldn't be replaced by pyparsing, it would just replace this class:
But all the Selection objects above would probably be 90% the same. That said, I had a play with it yesterday and didn't really get it immediately? So my current state of mind is, if it ain't broke don't fix it. |
Unless there's a compelling reason to switch, I say we stick with what we have. As long as new selection features don't have weird consequences/inconsistencies, I'm good with them. @kain88-de do you know anything we're missing out on without pyparsing? |
Otherwise the idea of @richardjgowers is nice. |
Updated LogicOperation metaclass to py2/3 style
Allows selections such as: 'resid 1:10 20:30' == 'resid 1:10 or resid 20:30'
…_atoms AG version is surely used more often?
Ok, so range selections now work with implicit OR, (see resid example in top comment) Also some overdue docstring love |
Very nice work here! I'll go through it in detail. |
Technically, this can't go into 0.13.1 --- it's a new feature so we have to increase the minor version and hence it must be 0.14.0. Ultimately, only the release manager @richardjgowers has to remember to make the "0.13 bugfixes" milestone actually a 0.14.0 release (and the #363 topology-rewrite will then become 0.15.0). Nice work here. VMD has these range selections and I find them very nice. |
segid *seg-name* | ||
select by segid (as given in the topology), | ||
e.g. ``segid 4AKE`` or ``segid DMPC`` | ||
altloc *alternative-location* |
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.
I just realized how the new topology system might make documenting the selection language right here difficult. Completely fine for this PR, but we'll have to think about this. I imagine it might be as simple as saying "the token is the singular attribute name, followed by values to filter on."
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.
Unless we hack __doc__
on the fly too?
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.
Yeah, we'll have to figure something out. Not sure yet.
Looking really good. A few questions to resolve and then we'll merge. |
Fixed missing group error, now raises SelectionError rather than missing token error
Conflicts: package/CHANGELOG
Cool, should be good to go |
Implicit or in selections (Issue #345) Allows selections such as: ``` python u.select_atoms('name C* N* O*') # == u.select_atoms('name C or name N') u.select_atoms('name N and resname GLY LEU') # == 'name N and (resname GLY or resname LEU)' u.select_atoms('resid 1:10 14 15') u.select_atoms('resid 1:100 200-300 and not resname MET GLY') ``` Defines how to detect keywords in selection strings with `is_keyword` (Issue #347).
Allows selections such as:
Defines how to detect keywords in selection strings with
is_keyword
(Issue #347).I'm not sure we agreed we wanted these, but it turned out to be pretty easy in the end.