FIX: Don't crash on non-string BIDS filter values - #3641
Conversation
Replace 'not isinstance(value, Query)' with 'isinstance(value, str)' in
the bids-filter unserialize step. The previous condition reached
'Query' in value for any non-Query input, which raised TypeError for
ints and floats (e.g. {"inv": 2} in a --bids-filter-file).
Closes niprepsgh-3394.
|
Thanks for opening this pull request! It looks like this is your first time contributing to fMRIPrep. 😄 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3641 +/- ##
=======================================
Coverage 74.57% 74.57%
=======================================
Files 62 62
Lines 4975 4975
Branches 637 637
=======================================
Hits 3710 3710
Misses 1131 1131
Partials 134 134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Closes #3394.
Changes proposed in this pull request
not isinstance(value, Query)withisinstance(value, str)in_process_value(fmriprep/config.py, called during--bids-filter-fileunserialization).Why
_process_valuewas checking'Query' in valuefor any non-Queryinput, which raisedTypeError: argument of type 'int' is not iterablewhenever a filter value was anintorfloat(e.g.{"inv": 2}in the JSON filter file). Usingisinstance(value, str)as the guard skips the substring check for non-string scalars, so they pass through unchanged.The
Query-passthrough behavior previously provided bynot isinstance(value, Query)is preserved naturally:Queryenum members are not instances ofstr, so the guard short-circuits and the value is returned unchanged.Testing
int,float,bool, plain strings, lists with mixed scalar types,repr(Query.OPTIONAL)(the existing Query-repr unserialize path), and existingQueryinstances. All pass through or unserialize as expected.ruff check,ruff format --check, andcodespellpass on the modified file.Documentation that should be reviewed
None.