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

bids_filter null values broken by fmriprep.config #2176

Closed
bpinsard opened this issue Jun 5, 2020 · 1 comment · Fixed by #2177
Closed

bids_filter null values broken by fmriprep.config #2176

bpinsard opened this issue Jun 5, 2020 · 1 comment · Fixed by #2177
Labels
BIDS bug effort: medium Estimated medium effort task impact: medium Estimated medium impact task
Milestone

Comments

@bpinsard
Copy link
Collaborator

bpinsard commented Jun 5, 2020

What version of fMRIPrep are you using?

20.1.0

What kind of installation are you using? Containers (Singularity, Docker), or "bare-metal"?

Singularity pulled from dockerhub.

What is the exact command-line you used?

<place your command line here>

NA

Have you checked that your inputs are BIDS valid?

NA

Did fMRIPrep generate the visual report for this particular subject? If yes, could you share it?

NA

Can you find some traces of the error reported in the visual report (at the bottom) or in crashfiles?

NA

Are you reusing previously computed results (e.g., FreeSurfer, Anatomical derivatives, work directory of previous run)?

NA

fMRIPrep log

If you have access to the output logged by fMRIPrep, please make sure to attach it as a text file to this issue.

NA

BUG

I am using a bids-filter file containing:

{
  "t1w": {
    "reconstruction": null,
    "acquisition": null
  }
}

But after runnning fMRIPrep, in the config file the corresponding section is empty:

[execution.bids_filters.T1w]

I tested toml.dumps with a similar dict, and it seems that it does not dump any dict entry with value None.

In [3]: toml.dumps(dict(t1w=dict(acquisition=None)))                                       
Out[3]: '[t1w]\n'                                                                        
                                                                                          
In [4]: toml.dumps(dict(t1w=dict(acquisition=1)))                                                  
Out[4]: '[t1w]\nacquisition = 1\n'                                                                   
                                                                                        

I don't know if this is a toml issue, or should we write an encoder/decoder that allows None/null values.

The other option would be to replace None with pybids.value (as it is done for bids.layout.Query.ANY) which dumps with toml:

In [8]: toml.dumps(dict(t1w=dict(acquisition=bids.layout.Query.NONE)))
Out[8]: '[t1w]\nacquisition = "<Query.NONE: 1>"\n'

There might be other cases when storing null value is significant.

@bpinsard bpinsard added the bug label Jun 5, 2020
@oesteban oesteban added this to the 20.1.2 milestone Jun 5, 2020
@bpinsard
Copy link
Collaborator Author

bpinsard commented Jun 5, 2020

might be worth adding a dump-load-compare test of a config with a bids-filters with edge cases such that one.

@oesteban oesteban added BIDS effort: medium Estimated medium effort task impact: medium Estimated medium impact task labels Jun 11, 2020
@mgxd mgxd closed this as completed in #2177 Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BIDS bug effort: medium Estimated medium effort task impact: medium Estimated medium impact task
Projects
None yet
2 participants