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

Allow for more flexibility with wildcard in selections #2436

Closed
jbarnoud opened this issue Jan 4, 2020 · 9 comments · Fixed by #2551
Closed

Allow for more flexibility with wildcard in selections #2436

jbarnoud opened this issue Jan 4, 2020 · 9 comments · Fixed by #2551

Comments

@jbarnoud
Copy link
Contributor

jbarnoud commented Jan 4, 2020

Is your feature request related to a problem? Please describe.

Selections strings allow for one wildcard that will glob any number of characters. This is surprising for advanced users of selections in VMD, for instance, that allows more flexible wildcards.

Describe the solution you'd like

Ideally, there should not be a limit in the number of wildcards that can be used. Also, the wildcard characters that can be used would be * for an undefined number of characters, . for one character, and ? for one or more characters.

Additional context

The fnmatch module from the standard library implements comparisons with such wildcards: https://docs.python.org/3.8/library/fnmatch.html

@orbeckst
Copy link
Member

orbeckst commented Jan 23, 2020

Would this be something for GSoC?

Especially if one couples it to a larger overhaul of selections (see #371 and #345 (comment) to use pyparsing) and oldies such as #104.

@jbarnoud
Copy link
Contributor Author

This alone is way too small for GSoC. If anything, I expect it to be a nice onboarding issue.

@richardjgowers
Copy link
Member

I'd disagree with @jbarnoud , I think this is a complicated project if we include more regex-like characters, so could be a gsoc project. It's worth keeping in mind that we've going to pass in n_atoms strings to any match function, so performance could be an issue too.

@jbarnoud
Copy link
Contributor Author

If it comes with a selection overhaul, it is indeed a more substantial project. But limited to the wildcards, it is a matter of changing the vicinity of

if wc_pos == -1: # No wildcard found
to call https://docs.python.org/3.8/library/fnmatch.html#fnmatch.filter instead.

@orbeckst
Copy link
Member

This limited issue seems to be good issue for someone to get into it. I will add a GSoC project with a proposed rewrite of the selection system.

@jimboH
Copy link

jimboH commented Feb 22, 2020

Hi, I am Hsueh, Cheng-Hsun, a medical student at National Yang-Ming University in Taiwan. I am interested in the 'Improved atom selections' project in GSoC 2020. Could I work on this issue?

@orbeckst
Copy link
Member

We are about to publish a blog post at https://www.mdanalysis.org/blog/ with more details for GSoC.

However, you're more than welcome to start working on this issue. Put up a pull request that references this issue. We can then comment and discuss there.

Note that other people are still free to work on this issue if they so desire. Ultimately, we want GSoC candidates to engage with our code and us as a community and the best way is to work on some code.

orbeckst pushed a commit that referenced this issue Mar 4, 2020
* Fixes #2436 
* Selection strings changed to use fnmatch. This now allows for more flexible wildcard usage as well as for using multiple wildcards at once.
* Added two new tests to match the new functionality
* New doc section on wildcards
* Remove test for multiple wildcards
* Update in AUTHORS
* Update CHANGELOG
@orbeckst
Copy link
Member

orbeckst commented Mar 4, 2020

@lilyminium with the updated wildcard matching, the section of the User Guide needs an update. I didn't require this as part of this PR (keeping it simple for GSoC). I will raise a UG issue.

@lilyminium
Copy link
Member

Thanks @Iv-Hristov and @orbeckst!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants