pybids
pybids copied to clipboard
Expanding derivatives' configuration to validate existing BIDS-apps
Under the assumption that the main goal of this configuration file is to be able to query BIDS-apps' outputs, I think it would be great to validate the correspondence between the described deafult_path_patterns
(and entities) and the actual outputs of these apps.
I've added some configurations that make it possible to query QSIPrep's derivatives, but I believe that people more familiar with possible outputs of this (and other) BIDS-app(s) will be able to make this even more robust.
Thanks for the PR and joining me in the "joyful" process of editing those config file manually.
So I think the assumption is partly right.
The config can help parse the outputs of bids Apps but only for the entities and filename patterns that are part of the bids specs. So far the content of the config is limited to that (actually a subset of what is in the spec - see below). So I think some of that PR goes beyond what we would want to have in those config.
Another reason why we may not want to add some of those changes is that we ultimately want to be able to automatically generate those config from the bids schema: when that happens that would destroy all the hard manual work of creating a config that works with bids apps that create outputs not supported by the bids schema.
That being said...
All the segmentation derivatives that are in the spec are not in the config and those should go in. I have been meaning to work on those for too long actually. But it would be better if we added some tests to make make sure the configs are correct. Most likely using the fmriprep dataset from the bids examples.
Another thing that could be nice is to have a separate config file for a given bids app so that users can easily load that config.
But I am not sure if it makes sense to have them shipped with pybids: maybe a separate repo in the bids organization?
I would encourage consideration of a separate repository for BIDS Apps support and for BIDS Transformations as well. BIDS Apps don't have to be in python do they? Neither do BIDS Transformations.
A more language-independent approach would be useful.
On Wed, Aug 3, 2022 at 10:59 AM Remi Gau @.***> wrote:
Thanks for the PR and joining me in the "joyful" process of editing those config file manually.
So I think the assumption is partly right.
The config can help parse the outputs of bids Apps but only for the entities and filename patterns that are part of the bids specs. So far the content of the config is limited to that (actually a subset of what is in the spec - see below). So I think some of that PR goes beyond what we would want to have in those config.
Another reason why we may not want to add some of those changes is that we ultimately want to be able to automatically generate those config from the bids schema: when that happens that would destroy all the hard manual work of creating a config that works with bids apps that create outputs not supported by the bids schema.
That being said...
All the segmentation derivatives that are in the spec are not in the config and those should go in. I have been meaning to work on those for too long actually. But it would be better if we added some tests to make make sure the configs are correct. Most likely using the fmriprep dataset from the bids examples.
Another thing that could be nice is to have a separate config file for a given bids app so that users can easily load that config.
But I am not sure if it makes sense to have them shipped with pybids: maybe a separate repo in the bids organization?
— Reply to this email directly, view it on GitHub https://github.com/bids-standard/pybids/pull/881#issuecomment-1204149158, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJCJOUK2OHIITDYWNLCRWDVXKJNJANCNFSM55N4PYPQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Yeah I am having some thoughts to maybe move towards using configurations for bids matlab so that would be useful for that reason too.
Thank you for the prompt reply!
Thanks for the PR and joining me in the "joyful" process of editing those config file manually.
So I think the assumption is partly right.
The config can help parse the outputs of bids Apps but only for the entities and filename patterns that are part of the bids specs. So far the content of the config is limited to that (actually a subset of what is in the spec - see below). So I think some of that PR goes beyond what we would want to have in those config.
Another reason why we may not want to add some of those changes is that we ultimately want to be able to automatically generate those config from the bids schema: when that happens that would destroy all the hard manual work of creating a config that works with bids apps that create outputs not supported by the bids schema.
That actually sounds great, I can understand why this PR is therefore somewhat irrelevant. Would love to contribute to the process of automating these path patterns :smile:
That being said...
All the segmentation derivatives that are in the spec are not in the config and those should go in. I have been meaning to work on those for too long actually. But it would be better if we added some tests to make make sure the configs are correct. Most likely using the fmriprep dataset from the bids examples.
On it!
Another thing that could be nice is to have a separate config file for a given bids app so that users can easily load that config.
But I am not sure if it makes sense to have them shipped with pybids: maybe a separate repo in the bids organization?
I would encourage consideration of a separate repository for BIDS Apps support and for BIDS Transformations as well. BIDS Apps don't have to be in python do they? Neither do BIDS Transformations. A more language-independent approach would be useful.
@Remi-Gau and @VisLab, funny you should mention this, I've started working on such a repo (although it's python-focused), but it's still in a very early stage. Would love to join forces if it's something that seems beneficial to you.
@Remi-Gau and @VisLab, funny you should mention this, I've started working on such a repo (although it's python-focused), but it's still in a very early stage. Would love to join forces if it's something that seems beneficial to you.
Let's chat when I am back from holidays but I think that having a repo where people can submit config to help work with some bids apps could be of great value for the community.
Codecov Report
Merging #881 (fd903a1) into master (1fa8382) will not change coverage. The diff coverage is
n/a
.
:exclamation: Current head fd903a1 differs from pull request most recent head be57b4d. Consider uploading reports for the commit be57b4d to get more accurate results
@@ Coverage Diff @@
## master #881 +/- ##
=======================================
Coverage 86.44% 86.44%
=======================================
Files 35 35
Lines 3992 3992
Branches 1038 1038
=======================================
Hits 3451 3451
Misses 350 350
Partials 191 191
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
@GalBenZvi
If you want to look at the tests for new configs on bids examples datasets
https://github.com/bids-standard/pybids/blob/fd903a14954ee741f540eea56771030534ccd311/bids/layout/tests/test_path_building.py#L52
https://github.com/bids-standard/pybids/blob/fd903a14954ee741f540eea56771030534ccd311/bids/layout/tests/test_layout_on_examples.py#L12
Let's chat when I am back from holidays but I think that having a repo where people can submit config to help work with some bids apps could be of great value for the community.
Sure, have fun!
@GalBenZvi
If you want to look at the tests for new configs on bids examples datasets
https://github.com/bids-standard/pybids/blob/fd903a14954ee741f540eea56771030534ccd311/bids/layout/tests/test_path_building.py#L52
https://github.com/bids-standard/pybids/blob/fd903a14954ee741f540eea56771030534ccd311/bids/layout/tests/test_layout_on_examples.py#L12
I'm actually kind of lost here... It seems that the current configuration isn't able to reconstruct paths of fmriprep's outputs. I was thinking about parsing these outputs to their entities and then trying reconstructing the origin paths as a test for both the entities and default path pattern, but it doesn't seem like it's currently possible.
Obviously, a way to work around it is editing the configuration to capture these outputs, but I'm not sure if it's the right way to go following your notes.
I'm actually kind of lost here... It seems that the current configuration isn't able to reconstruct paths of fmriprep's outputs.\nI was thinking about parsing these outputs to their entities and then trying reconstructing the origin paths as a test for both the entities and default path pattern, but it doesn't seem like it's currently possible.
Sorry. I think I was unclear.
You are correct and that's because some files from fmriprep use entities that are yet part of the official spec.
So the idea would be to test the path builing only of the files that contain entities present in the spec. And skip the others.
This would also mean that one of the "low" hanging fruit for a config in the other repo you were mentioning would be to add an config for fmriprep that covers all the files it can generate.
here is how the "skipping of non covered entities" is done in another test
https://github.com/bids-standard/pybids/blob/fd903a14954ee741f540eea56771030534ccd311/bids/layout/tests/test_path_building.py#L81