pybids 0.6.4 causes test failure in BIDSDataGrabber

[effigies]

Summary

The error in https://travis-ci.org/nipy/nipype/builds/408021599:

______________________________ test_bids_grabber _______________________________
[gw0] linux -- Python 3.5.5 /home/travis/virtualenv/python3.5.5/bin/python
tmpdir = local('/tmp/pytest-of-travis/pytest-0/popen-gw0/test_bids_grabber0')
    @pytest.mark.skipif(not have_pybids,
                        reason="Pybids is not installed")
    @pytest.mark.skipif(sys.version_info < (3, 0),
                        reason="Pybids no longer supports Python 2")
    @pytest.mark.skipif(not dist_is_editable('pybids'),
                        reason="Pybids is not installed in editable mode")
    def test_bids_grabber(tmpdir):
        tmpdir.chdir()
        bg = nio.BIDSDataGrabber()
        bg.inputs.base_dir = os.path.join(datadir, 'ds005')
        bg.inputs.subject = '01'
        results = bg.run()
        assert os.path.basename(results.outputs.anat[0]) == 'sub-01_T1w.nii.gz'
>       assert os.path.basename(results.outputs.func[0]) == (
            'sub-01_task-mixedgamblestask_run-01_bold.nii.gz')
E       AssertionError: assert 'sub-01_task-...01_events.tsv' == 'sub-01_task-m...1_bold.nii.gz'
E         - sub-01_task-mixedgamblestask_run-01_events.tsv
E         ?                                     ^^^ ^^ ^^^
E         + sub-01_task-mixedgamblestask_run-01_bold.nii.gz
E         ?                                     ^^^^^ ^^ ^^
/home/travis/build/nipy/nipype/nipype/interfaces/tests/test_io.py:596: AssertionError

@adelavega @tyarkoni Flagging you in this in case the cause is obvious.

It's not immediately clear whether the correct fix will be in pybids or nipype.

Execution environment

Travis CI

[effigies]

Okay, it's indexing the derivatives first, so it's the new test data that's causing the issue.

In [1]: import os

In [2]: from nipype.interfaces import io as nio

In [3]: from nipype.interfaces.tests.test_io import datadir

In [4]: bg = nio.BIDSDataGrabber()

In [5]: bg.inputs.base_dir = os.path.join(datadir, 'ds005')

In [6]: bg.inputs.subject = '01'

In [7]: results = bg.run()

In [8]: results.outputs.func
Out[8]: 
['.../bids/tests/data/ds005/derivatives/events/func/sub-01_task-mixedgamblestask_run-01_events.tsv',
 '.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-01_bold.nii.gz',
 '.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-01_events.tsv',
 '.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold.nii.gz',
 '.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-02_events.tsv',
 '.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-03_bold.nii.gz',
 '.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-03_events.tsv']

Now to see whether it's pybids incorrectly indexing derivatives or nipype incorrectly calling BIDSLayout...

[effigies]

@tyarkoni I think this is a pybids problem. Opening an issue over there.

In [9]: import bids

In [10]: layout = bids.BIDSLayout(os.path.join(datadir, 'ds005'))

In [11]: layout.get(subject='01', task='mixedgamblestask', modality='func')
Out[11]: 
[File(filename='.../bids/tests/data/ds005/derivatives/events/func/sub-01_task-mixedgamblestask_run-01_events.tsv', subject='01', task='mixedgamblestask', run='1', type='events', modality='func'),
 File(filename='.../bids/tests/data/ds005/sub-01/func/sub-01_task-mixedgamblestask_run-01_bold.nii.gz', subject='01', task='mixedgamblestask', run='1', type='bold', modality='func'),
 ...]

[adelavega]

I think the test is actually what's broken. I updated the test data so that the event file would also have modality = 'func'. Thus, the output you list at the end is correct.

I think the issue is that this line should also have "type": "bold" to disambiguate from event files, if the intention is for only bold files and anat files (without event files) to be indexed.

       self.inputs.output_query = {"func": {"modality": "func"},
                                    "anat": {"modality": "anat"}}

[adelavega]

After reading the issue over in pybids, I think what was happening is that since we were only checking the first output in the test, we didnt realize all the event files were also being indexed, which is not the intended behavior (correct? should only be bold files). The change I made to the bids test data (which added modality=func to the derivatives event file) only made us aware of this already existing problem.

[effigies]

As far as I'm concerned, it's valid to grab any files. If you want niftis, then set it as a parameter to the interface.

…

On Wed, Jul 25, 2018, 12:20 Alejandro de la Vega ***@***.***> wrote: After reading the issue over in pybids, I think what was happening is that since we were only checking the first output in the test, we didnt realize all the event files were also being indexed, which is *not* the intended behavior (correct? should only be bold files). Indexing the derivatives first just made us aware of the problem. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2650 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFF8jnssoFh4rk6x99om4QxWxZZB6b8ks5uKJrmgaJpZM4Vf_wy> .

[adelavega]

I agree, but I thought the default behavior was to only grab bold and anat images. Of course you can change the defaults and grab whatever you want, but I think that was the intention at least.

It doesn't make much sense to me as a default to grab both bolds and event files mixed together in the same output field.

[adelavega]

While we're at it, the test also depends on the outputs being returned in a specific order, which is not necessarily guaranteed I don't think. So maybe its best to check if sub-01_task-mixedgamblestask_run-01_bold.nii.gz in results.outputs.func , rather than if its the first element.

Also probably good to check the number of results though, to check the right files are being indexed.

[adelavega]

See the docstring for intended default behavior:

By default, the BIDSDataGrabber fetches anatomical and functional images
from a project, and makes BIDS entities (e.g. subject) available for
filtering outputs.

[effigies]

Okay do you want to submit a PR?

…

On Wed, Jul 25, 2018, 12:31 Alejandro de la Vega ***@***.***> wrote: While we're at it, the test also depends on the outputs being returned in a specific order, which is not necessarily guaranteed I don't think. So maybe its best to check if sub-01_task-mixedgamblestask_run-01_bold.nii.gz in results.outputs.func , rather than if its the first element. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2650 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFF8oAtqaMOnhrgmdUSg7OPfHTYsD3Gks5uKJ1XgaJpZM4Vf_wy> .