-
Notifications
You must be signed in to change notification settings - Fork 3
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
Dealing with derivatives #45
Conversation
raise NotADirectoryError( | ||
f"The input path is not valid. You specified '{input_path}', which is not a directory.") | ||
|
||
# if not(BIDSValidator().is_bids(input_path)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete commented lines or specify what the missing TODO / FIXME is for those lines
) | ||
return bids_layout, layout_df, dataset_description | ||
|
||
if ("DatasetType" in dataset_description) and (dataset_description["DatasetType"] == "derivative"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since dataset_description["DatasetType"]
is only RECOMMENDED this is not a reliable path to check for derivatives.
I would suggest to only go with line 57-66 to determine if derivatives are present. This only works though in the case of derivatives that are provided in a subdirectory of a BIDS raw dataset. In case only derivative data are shared as BIDS dataset, there is no requirement to store those data in a "derivative" subdirectory (if I understand the specifications correctly).
However you can make use of the REQUIREMENT that the dataset_description.json for derivative data MUST have a "GeneratedBy"
key specified.
Also note that if derived datasets are stored in a subdirectory of a raw dataset in BIDS, there are two dataset_description.json files: one for the raw data and one for (each) derived dataset.
return bids_layout, layout_df, dataset_description | ||
|
||
if ("DatasetType" in dataset_description) and (dataset_description["DatasetType"] == "derivative"): | ||
bids_layout = BIDSLayout(input_path, is_derivative=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only loading derivatives or all (raw + derivatives)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume all
layout_df = bids_layout.to_df() | ||
return bids_layout, layout_df, dataset_description | ||
|
||
bids_layout = BIDSLayout(input_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why you need the upper lines. Based on the specifications, would it not be sufficient enough to state
has_derived_data = False:
if os.path.isdir(os.path.join(input_path, "derivatives")) or
"GeneratedBy" in dataset_description:
has_derived_data = True
bids_layout = BIDSLayout(input_path, is_derivative=has_derived_data)
layout_df = bids_layout.to_df()
?? (please double check the code, not sure I wrote this correctly)
Note that the data types also need to be set in openMINDS (can be an array). Not sure if you do this later somewhere?
@@ -5,7 +5,8 @@ | |||
import bids2openminds.converter | |||
|
|||
example_dataset = [("ds003", 13), ("ds000247", 6), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make a comment and explain what the numbers are in the example dataset tuples?
Postponed to later, see issue #54 |
This pull request verify if the dataset contains or derivatives or is derived dataset, before calling BIDSLayout.