-
Notifications
You must be signed in to change notification settings - Fork 130
.get()'s behavior options can possibly conflict with query terms (entities and metadata fields) #397
Description
ATM get's signature is
def get(self, return_type='object', target=None, extensions=None,
scope='all', regex_search=False, defined_fields=None, **kwargs):which says that I cannot .get anything based on entities or metadata with fields which are get options, such as "target", "scope", etc. Even if ok now, any possible extension of get signature should be done really carefully (if at all) since might cause existing code using it break if such a new collision happen.
In the long(er) run I think it would be better to avoid such conflicts, so pybids API does not limit what BIDS terms etc could express. Possible solutions I see but may be there could be better ones
- define
.gettter(the same signature as ofgetbut without**kwargs) which would return the actual function to be called with**kwargs.getshould then should just accept**kwargsonly and implemented as a lean wrapper aroundreturn self.getter()(**kwargs)). May be for a whilegetcould still accept any of the current options, complain about future deprecation, and pass them into thegetter. Suggest better options forgettername ;)- a variant of this is to define a new (
__call__) method which would provide the clean version ofget(**kwargs)and slowly deprecategetitself
- a variant of this is to define a new (
- extend
getsignature with an explicitquery=Noneargument which would optionally take the value of**kwargsas a dictionary. May be while at it even could be replaced with a splitmetadata=entities=since now there is an internal battle (entities win) in case of a collision. Users then can just place their query arguments within adict()to retain easy'ish way to specify, e.g.get(query=dict(suffix='T1w', return_type='file'))
But either of those two solutions would be incomplete if get with **kwargs would remain the documented/preferred way to query for files etc. So ideally get(..., **kwargs) way should get deprecated.
What do you think ?