-
Notifications
You must be signed in to change notification settings - Fork 87
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
read_raw_bids can not infer 'task' even if BIDSPath can? #1014
Comments
Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽♂️ |
you suggest that it should guess the task key if only one task is present?
even if you do not explicitly provide it?
… Message ID: ***@***.***>
|
yes, like BIDSpath does. right now I am passing a BIDSpath object which prints as E:/dataMoving/EESM1a_kaare/derivatives/cleaned_1/sub-002/ses-001/eeg/sub-002_ses-001_task-sleep_eeg.set which is the file I have in mind. However, read_raw_bids will complain that this file does not exist: E:\dataMoving\EESM1a_kaare\derivatives\cleaned_1\sub-002\ses-001\eeg\sub-002_ses-001_eeg.set This is also correct, but irrelevant, since I was asking for the one with "task=sleep". BIDSPath knows this, so why does read_raw_bids not? Another solution would be to have BIDSPath be less intelligent, and simply complain that no such file exists, instead of guessing the task. But that seems a downgrade, when the solution is simple to implement? |
This comment was marked as duplicate.
This comment was marked as duplicate.
If BIDSPath infers the task then I believe we should classify this behavior (inferring the task) as a bug and drop this. The task is a required BIDS entity and if not present, we should not try to be clever. We've tried just that (being clever) many times in the past and it leads to breakage down the road, sometimes sooner, sometimes later. |
I am also inclined to consider that guess the task is black magic and
likely to produce issues in many cases. I would just drop this
option.
… Message ID: ***@***.***>
|
Another issue I just ran into with regards to BIDSPath trying to infer the full path is when extension is specified, but suffix is not. There is no error and the full path can't be written out. I think in general, is there a BIDS spec on what MUST be specified in a path and what is optional? Then we can I think refactor the BIDSPath based on this and also write error messages to indicate what entities are missing from a user. |
I only know it for MEG and EEG from the top of my head, but there the minimum you need is:
|
Describe the bug
If a recording has a 'task' key, but the value is implicit, read_raw_bids is unable to read it. This is different from BIDSPath, which is able to infer it if there is only one possibility.
Steps to reproduce
code:
This will work:
Output:
(I have excluded a lot of output from read_raw_bids())
This won't:
Output:
Expected results
If BIDSPath() can infer the task, that logic should be utilized by read_raw_bids() as well. Instead of reading the field values directly from the BIDSPath object, read_raw_bids() should use get_entities_from_fname(). Then whatever smart logic is hidden in BIDSpath can be reused by read_raw_bids(). When I call mb.get_entities_from_fname(str(filePath)), it retrieves all the correct information.
Actual results
If task is not explicitly set, read_raw_bids() fails.
The text was updated successfully, but these errors were encountered: