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

Issues with bids.internal.get_metadata for derivatives #175

Closed
bmdeen opened this issue Feb 24, 2021 · 5 comments · Fixed by #179
Closed

Issues with bids.internal.get_metadata for derivatives #175

bmdeen opened this issue Feb 24, 2021 · 5 comments · Fixed by #179
Labels
bug Something isn't working

Comments

@bmdeen
Copy link
Collaborator

bmdeen commented Feb 24, 2021

I'm encountering issues using bids.internal.get_metadata to extract metadata for derivative files. Specifically, I have a directory of Freesurfer-derived surface files, each with their own json file, including:

sub-01_hemi-R_space-fsnative_den-native_thickness.shape.gii
sub-01_space-fsnative_den-native_thickness.dscalar.nii
sub-01_hemi-R_space-fsnative_den-native_midthickness.surf.gii

The first file is a GIFTI file specifying cortical thickness; the second file is a CIFTI file combining cortical thickness measures from left and right hemispheres; the third is a representation of the right cortical surface (mid-thickness).

When I use bids.internal.get_metadata (default pattern) on the first file, information from the json metadata for files 2 and 3 is included, as if all of these json files apply to the first file.

The file sub-01_space-fsnative_den-native_thickness.json is included because its key/value pairs are a subset of the first file's. So get_metadata is properly applying the bids standard here, but the standard behaves weirdly in this case. Not sure how to address this.

The file sub-01_hemi-R_space-fsnative_den-native_midthickness.json is included because the file matches the pattern ^.*thickness\.json$, and its key-value pairs are the same as the first file's. Here, the bids standard is not being properly applied - the midthickness json data should not be included.

@Remi-Gau
Copy link
Collaborator

Hey @bmdeen,

get_metadata is one of the thing that is on my plate for the end of this week (See PR #172 ). So I will add an extra portion of "thing to check" with this. 😄

And there has been a couple of issues that have been needing fixing for the metadata aspect since like FOR freaking EVER.

Just to know: you are just using the get_metadata function as a "standalone" and not through the call to layout, correct?

Also you are using the master branch version of bids-matlab, or the dev branch?

Last question: do you know if there is a dataset available somewhere where I could test that on? Otherwise no worry I will try to create some dummy files to test that on.

@bmdeen
Copy link
Collaborator Author

bmdeen commented Feb 25, 2021

Sounds good, thanks.

I'm using get_metadata as a standalone, from the master branch.

I put some of the data I'm having issues with in a folder here: https://www.dropbox.com/sh/7dw1x0h0txwv5fu/AACzARARw1MWFHCoxdlNnoWla?dl=0

@Remi-Gau
Copy link
Collaborator

OK so here are the tests I would set up. They indeed fail.

    data_dir = fullfile(fileparts(mfilename('fullpath')), 'data', 'SurfaceData');
    
    file = fullfile(data_dir, 'sub-06_hemi-R_space-individual_den-native_thickness.shape.gii');
    side_car = fullfile(data_dir, 'sub-06_hemi-R_space-individual_den-native_thickness.json');
    metadata = bids.internal.get_metadata(file);
    expected_metadata = bids.util.jsondecode(side_car);
    assertEqual(metadata, expected_metadata)
    
    file = fullfile(data_dir, 'sub-06_hemi-R_space-individual_den-native_midthickness.surf.gii');
    side_car = fullfile(data_dir, 'sub-06_hemi-R_space-individual_den-native_midthickness.json');
    metadata = bids.internal.get_metadata(file);
    expected_metadata = bids.util.jsondecode(side_car);
    assertEqual(metadata, expected_metadata)    
    
    file = fullfile(data_dir, 'sub-06_space-individual_den-native_thickness.dscalar.nii');
    side_car = fullfile(data_dir, 'sub-06_space-individual_den-native_thickness.json');
    metadata = bids.internal.get_metadata(file);
    expected_metadata = bids.util.jsondecode(side_car);
    assertEqual(metadata, expected_metadata)  

The fix is to change the default pattern to this

pattern = '^.*_%s\\.json$'

instead of

pattern = '^.*%s\\.json$'

Otherwise, the suffixes thickness and midthickness are not distinguishable.

This makes the default a bit less general but given that _suffix.extension is the rule in the vast majority of cases, I am tempted to go with it.

Will open a PR for this.

@bmdeen
Copy link
Collaborator Author

bmdeen commented Feb 26, 2021

Makes sense. You could also check for files that have no underscores (i.e. just suffix.extension) and use a different pattern in this case, to include those json files.

@Remi-Gau
Copy link
Collaborator

Makes sense. You could also check for files that have no underscores (i.e. just suffix.extension) and use a different pattern in this case, to include those json files.

ah yeah.
thanks!!! not sure why I did not think of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants