inheritance principle - Test failure - metadata picking up per-subject data items

[apjanke]

In the tests/test_get_metadata.m file, there are some commented-out tests.

% define the expected output from bids query metadata
func.RepetitionTime = 7;
func_sub_01.RepetitionTime = 10;
anat.FlipAngle = 5;
anat_sub_01.FlipAngle = 10;
anat_sub_01.Manufacturer = 'Siemens';

[...]

%% test func metadata base directory
metadata = bids.query(BIDS, 'metadata', 'type', 'bold');
%assert(metadata.RepetitionTime == func.RepetitionTime);


%% test func metadata subject 01
metadata = bids.query(BIDS, 'metadata', 'sub', '01', 'type', 'bold');
%assert(metadata.RepetitionTime == func_sub_01.RepetitionTime);


%% test anat metadata base directory
metadata = bids.query(BIDS, 'metadata', 'type', 'T1w');
%assert(metadata.FlipAngle == anat.FlipAngle);


%% test anat metadata subject 01
metadata = bids.query(BIDS, 'metadata', 'sub', '01', 'type', 'T1w');
assert(metadata.FlipAngle == anat_sub_01.FlipAngle);
assert(strcmp(metadata.Manufacturer, anat_sub_01.Manufacturer));

When I comment these asserts back in, the tests for the "base directory" cases fail. It looks like even when you're not passing the 'sub','01' filter to bids.query(), it still picks up the subject 01 metadata instead of the metadata from the task-auditory_bold.json file in the root of this BIDS directory. (E.g. RepetitionTime is 10 in both of the 'type','bold' queries, instead of 7 and 10, respectively.) Is this expected behavior?

[Remi-Gau]

Well it is not the expected behaviour. I think that one of the items on the to do list has been trying to make sure that the BIDS inheritance principle is properly implemented but we have gotten there yet.

[gllmflndn]

Yes, a valid implementation of the inheritance principle still requires some work. The commented out tests were created here: https://github.com/bids-standard/bids-matlab/pull/10 Note that, at least until recently, the inheritance principle was not entirely clearly defined (especially its granularity) so it might be worth checking its status.

[apjanke]

I may be able to help with this.

It looks like bids.query, when you don't pass a 'sub','01' query filter, filters to "all subjects" and still goes down to the subject level when searching for metadata. And that subject-level metadata overrides the top-level metadata.

When there's no 'sub' filter, should query be looking at just the top-level metadata? Or still be pulling in metadata from all subjects? Or is this something I should be referring to the BIDS standard documentation for?

[Remi-Gau]

I would tend to say that in terms of behavior we should maybe align on what pybids does.

I think that here part our problem is that the way the metadata is collected by get_metadata, because from my understanding it does the opposite of what we want: it starts by the lowest level in the hierarchy and then goes up level after level and overrides the metadata by what ever it finds on the way.

Am I wrong about this?

[apjanke]

I think the get_metadata implementation is doing the Inheritance principle correctly already. It does start at the lowest level, and then moves up to higher levels, using update_metadata to add the higher-level metadata. But the key is this line, down in update_metadata:

    if ~isfield(s1,fn{i})
        s1.(fn{i}) = s2.(fn{i});
    end

That ~isfield means that the new (upper-level) metadata is only applied if no lower-level metadata source had already set it for that key. So lower-level metadata specifications override higher-level metadata on a per-key basis.

I think the issue with the commented-out tests is that the test code is acting like bids.query(BIDS, 'metadata') will directly get the higher-level metadata, when it actually goes and gets bottom-level per-subject/session metadata. If you want query to be able to directly get the higher-level metadata, it will need a special calling form for that.

I'll have a look at what pybids does here.