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

Use trailing undescore to disambiguate entity names from python keywords #947

Open
pvandyken opened this issue Feb 15, 2023 · 4 comments
Open

Comments

@pvandyken
Copy link
Contributor

Currently, the from entity, not yet officially part of BIDS spec afaik but still found withing the derivatives config in pybids, overlaps with the python keyword from, as in from bids import BIDSLayout. As a result, the following cannot be run without a syntax error:

layout.get(from="space", to="other_space")

The convention for such cases is to add a trailing underscore to the reserved word (see PEP8), e.g. from_. I propose we allow the use of such underscored arguments in the .get method (and wherever else it's relevant).

This could be on a entity-by-entity basis, but as underscores in entity names are not valid or sensible in BIDS anyway, I think it would safe enough to just remove trailing underscores from every filter passed to bids. This would cover any future cases (e.g if class was ever made an entity).

@pvandyken
Copy link
Contributor Author

It is possible to work around this using:

layout.get(**{"from": "space"})

but it's a super awkward syntax.

@pvandyken
Copy link
Contributor Author

If you're interested, I'm definitely able to implement this, but I likely won't have time in the near future

@effigies
Copy link
Collaborator

I think I would be okay with removing trailing underscores from filters, unconditionally. @adelavega?

@adelavega
Copy link
Collaborator

Can't think of why not

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

No branches or pull requests

3 participants