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

Customizable bids function based on spec file #245

Closed
pvandyken opened this issue Feb 11, 2023 · 5 comments · Fixed by #349
Closed

Customizable bids function based on spec file #245

pvandyken opened this issue Feb 11, 2023 · 5 comments · Fixed by #349
Labels
enhancement New feature or request

Comments

@pvandyken
Copy link
Contributor

pvandyken commented Feb 11, 2023

The proposal is to implement a new function as follows:

def bids_template(spec: Path | str | BidsPathSpec):
	"""Generate a bids function that constructs files based on the provided spec

	Parameters
	==========
	spec:
		Path to a json or yaml file containing the spec, or a BidsPathSpec shaped dict

	Returns
	=======
	Callable[[<bids args>], str]

Then, the existing bids function would be implemented as:

bids = bids_template("path/to/default/spec.json")

BidsPathSpec will dictate the structure of the spec file and will be as follows:

class BidsPathSpec(TypedDict):
	"""Expected structure for spec defining how bids paths shall be constructed
	
	For all entity lists below, the long and short versions of each entity name
	(e.g. "subject" vs "sub") shall be considered synonymous, IF the entity
	is listed in `bids_tags.json` (and possibly in the future, the `pybids` config
	file). Otherwise, the short version of the entity name MUST be used.
	"""

	directories: list[str]
	"""Entities to be used as folders.

	Each entity listed as a directory will be on its own fs level. Each entity may be listed
	AT MOST one time. `datatype` MAY be listed as a directory, but if it is not, it will be
	implicitly included as the last entry in the list. If given, its position in the order
	will be respected.

	Entities listed in `directories` MUST also be listed in `order`, EXCEPT for `datatype`,
	which may not be listed in `order`.
	"""

	order: list[str]
	"""Order of entities in the final path component

	Each entity may be listed AT MOST one time. NONE of `datatype`, `suffix`, `prefix`, and
	`extension` may be listed as entities. `"*"` MAY be listed as an entity, but if it is
	not, it will be implicitly included as the last entry in the list. Either way, its
	position marks the location where unrecognized entities will be inserted. Only entities
	explicitly listed in `order` will be considered "recognized". No information from pybids
	or other sources will be considered.

	All entities must follow the BIDS naming requirements for entity names: only alphanumeric
	characters (regex `r'[a-zA-Z0-9]+'`). `"*"`, the wildcard indicator, is of course the only
	exception.
	"""
@pvandyken pvandyken added the enhancement New feature or request label Feb 11, 2023
@pvandyken
Copy link
Contributor Author

@akhanf, @tkkuehn, @kaitj, I'd especially like feedback on the proposal to treat long and short versions of entity names as synonymous. I went this route because our current treatment of names is inconsistent: we use the long versions of subject and session, but the short versions of everything else. This is extra aggravating because pybids always uses the long version, meaning users must give the long version in their snakebids.yaml file. I felt the easiest way to tame this without breaking anything was just to allow both. But I'm open to other ideas.

@pvandyken
Copy link
Contributor Author

Obviously, feedback on any other aspect is also welcome, including on the proposed handling of datatype.

@kaitj
Copy link
Contributor

kaitj commented May 19, 2023

Just going through the list of issues and realized I missed this.

re: subject / session - I think the current implementation came from pybids and I don't know if there was a reason why it strictly uses long-form or these entities. That said, I don't see any downsides to your suggestion of using both long and short forms as long as it is passed properly to the associated entity in pybids as needed.

@pvandyken
Copy link
Contributor Author

Yeah, so my logic is that the pybids logic in .get() is to always use the long form, but snakebids.bids() default strategy is to use the key verbatim. So to use both long and short forms is to keep the existing policy, adding support for the pybids understanding of long-form names.

But there's a few other issues here too: I believe if someone does bids(acq="foo", sub="001"), sub will go to the end, as it's not recognized. This case should be handled correctly: there's no case where sub-xxx at the end of a bids path is correct.

@pvandyken
Copy link
Contributor Author

By the way, I've been waiting on this issue until we get bids moved over to a new repository. I suppose there's nothing stopping us making changes here, but completing the move seemed like a logical first step

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants