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

ENH: Adds populate_intended_for for fmaps #482

Merged
merged 94 commits into from
Feb 24, 2022
Merged
Changes from 1 commit
Commits
Show all changes
94 commits
Select commit Hold shift + click to select a range
6dbd28a
Adds `add_field_to_json` to utils.py
pvelasco Dec 9, 2020
d5f01b9
ENH: Adds `populate_intended_for`
pvelasco Dec 11, 2020
7117659
Adds `populate_intended_for` to `process_extra_commands`
pvelasco Dec 14, 2020
5c4ff83
BF: acq_str/run_str will not break if no acq/run label present
pvelasco Dec 17, 2020
77d63f1
RF: `populate-intended-for` command uses dashes (`-`)
pvelasco Dec 17, 2020
42d3c20
RF: Prepares way for further `populate_intended_for` unit tests
pvelasco Dec 17, 2020
0d821d5
Corrects usage of `populate-intended-for` in allowed commands
pvelasco Dec 17, 2020
7edcf36
Merge pull request #4 from nipy/master
pvelasco Dec 18, 2020
34b2acc
BF: Fixes broken test for `populate_intended_for` in Python3.5
pvelasco Dec 18, 2020
7a01b5f
RF: make `get_shim_setting` a separate function
pvelasco Dec 18, 2020
36643ae
RF: Simplify a little bit `create_dummy_pepolar_bids_session`
pvelasco Dec 19, 2020
1be873c
RF: `test_bids.py: create_dummy_pepolar_bids_session` now also return…
pvelasco Dec 21, 2020
7815c92
Adds tests for the case in which there are no ShimSettings
pvelasco Dec 21, 2020
4b21d45
BF: Fixes bug in `populate_intended_for`
pvelasco Dec 22, 2020
03d4005
Adds a new test case for `populate_intended_for`
pvelasco Dec 22, 2020
d3c9962
RF: `test_convert.py` imports full module `convert.py`
pvelasco Dec 23, 2020
1bd783a
ENH: Adds a call to `populate_intended_for` in `convert.py:convert`
pvelasco Dec 23, 2020
d9dabe7
BF: Fixes unit tests for test_convert.py
pvelasco Jan 4, 2021
fb7dd70
BF: Fixes unit tests for test_convert.py
pvelasco Jan 4, 2021
015f79c
BF: skips populate_intended_for for non-BIDS-compliant datasets
pvelasco Jan 5, 2021
90ccae5
Merge 'nipy_heudiconv/master' into adds_populate_intended_for
pvelasco Feb 4, 2021
0690c1a
Update heudiconv/bids.py
pvelasco Feb 18, 2021
9b397de
Improve lgr in `bids.py`
pvelasco Feb 18, 2021
fefa521
Merge remote-tracking branch 'nipy_heudiconv/master' into adds_popula…
pvelasco Feb 18, 2021
d801fee
Simplify sorting of fmaps in heudiconv/bids.py
pvelasco Feb 19, 2021
8f25fb6
Simplify expression to find session field maps
pvelasco Mar 2, 2021
b9a1d7a
Minor: Use preferred str formatting in lgr call
pvelasco Mar 2, 2021
37c9b7a
RF: Add find_fmap_groups for populating IntendedFor
pvelasco Mar 10, 2021
cfcb118
Add new test case for find_fmap_groups
pvelasco Mar 10, 2021
6027ab2
ENH: New function to extract info relevant for IntendedFor
pvelasco Mar 15, 2021
4672145
RF: populate_intended_for uses now get_key_info_for_fmap_assignment
pvelasco Mar 15, 2021
1aa5f2d
Add functions to find fmaps compatible with runs/sessions
pvelasco Mar 18, 2021
ec38175
RF: rename variables
pvelasco Mar 22, 2021
c05b8cf
RF: `find_compatible_fmaps_*` now return compatible_fmap_groups
pvelasco Mar 22, 2021
af80739
Add `select_fmap_from_compatible_groups`, allowing the user to select…
pvelasco Mar 31, 2021
2f9055b
Fix test_get_key_info_for_fmap_assignment
pvelasco Apr 1, 2021
3d85e2b
RF: improve `populate_intended_for`
pvelasco Apr 1, 2021
394d8f9
RF: Add `remove_prefix`/`remove_suffix` for path strings
pvelasco Apr 1, 2021
af65665
Merge nipy/heudiconv:master into this branch
pvelasco Apr 2, 2021
bf8f0a9
$BF: Fix test_convert.test_convert()
pvelasco Apr 2, 2021
b5b5051
Merge branch 'adds_populate_intended_for' into adds_populate_intended…
pvelasco Apr 2, 2021
f947173
RF, ENH: Expand choices for `populate_intended_for`
pvelasco Apr 2, 2021
5615f99
Merge branch 'master' into adds_populate_intended_for
pvelasco Apr 6, 2021
8811491
BF: Fix op.join(tmpdir,...) in test_bids for Python3.5 compatibility
pvelasco Apr 6, 2021
e6ba946
Merge remote-tracking branch 'origin/adds_populate_intended_for' into…
pvelasco Apr 6, 2021
1e1ba4d
BF: Make sure the order of the test files is the intended one
pvelasco Apr 8, 2021
a0ee48e
BF: Make sure we remove the trailing op.sep in path
pvelasco Apr 8, 2021
09e0d1d
BF: Fixes test_convert.test_convert
pvelasco Apr 16, 2021
c38253b
ENH: `convert` now takes `populate_intended_for` params, from heuristic
pvelasco Apr 16, 2021
fa7b809
RF: test_convert -> test_populate_intended_for
pvelasco Apr 20, 2021
47bab3c
RF: Get rid of sys_modules call
pvelasco Apr 20, 2021
5b0595a
Change format of debug logger.
pvelasco Apr 20, 2021
0650b68
RF: utils.add_field_to_json -> utils.update_json
pvelasco Apr 20, 2021
ce1f0d2
Don't specify intent in `save_json`
pvelasco Apr 20, 2021
c4a2224
RF: Allow user to specify `pretty` argument for update_json
pvelasco Apr 20, 2021
267bf62
BF: Delete duplicated functions in bids.py
pvelasco Apr 20, 2021
c56cba4
Add documentation for `populate_intended_for`
pvelasco Apr 29, 2021
c4c7587
ENH: do use update_json for IntendedFor addition with pretty=True
yarikoptic Apr 30, 2021
467a261
Merge remote-tracking branch 'origin/master' into adds_populate_inten…
yarikoptic May 3, 2021
ae61382
Revert to importing specific functions in test_convert.
pvelasco May 3, 2021
f3dd790
Merge remote into local for branch 'adds_populate_intended_for'
pvelasco May 3, 2021
723fa67
BF: call `populate_intended_for` using the full session path
pvelasco May 3, 2021
5a06034
RF: only call `populate_intended_for` if heuristic specifies the options
pvelasco May 6, 2021
1bd151b
Add test for populate_intended_for using heuristic without POPULATE_I…
pvelasco May 10, 2021
ccb96df
RF: allow multiple parameters that need to be matched for populate_in…
pvelasco May 10, 2021
b72ecd8
Make `'ImagingVolume'` the default matching_parameters
pvelasco May 13, 2021
34c1b41
Add `'Force'` as `matching_parameter` option for fmap matching
pvelasco May 14, 2021
bdbc91e
RF: Add `'AcquisitionLabel'` as `matching_parameter` option for fmap …
pvelasco May 14, 2021
156ed9b
ENH: pass POPULATE_INTENDED_FOR_OPTS from heuristic upon command styl…
Jun 11, 2021
bf9a3e1
BF: Various minor fixes while trying to use with nibabel 3.2.1
Jun 11, 2021
bda7cc5
Cover the case key_info is a string; modify unit tests
pvelasco Jun 21, 2021
606a727
Remove import namedtuple (no longer needed)
pvelasco Jun 21, 2021
2886525
ENH: Add BIDSFile class
pvelasco Jun 21, 2021
d3b8210
Merge pull request #11 from cbinyu/BIDSFile_helper
pvelasco Jun 21, 2021
141f6e9
Merge pull request #34 from cbinyu/patch_dbic
yarikoptic Jun 21, 2021
0ecd867
Merge pull request #10 from dbic/adds_populate_intended_for
pvelasco Jun 22, 2021
1867898
Remove default arguments for populate-intended-for functions
pvelasco Jul 19, 2021
f8c5528
Update docs/heuristics.rst for RF matching_parameters key
pvelasco Jul 19, 2021
312e139
Update heudiconv/bids.py
pvelasco Sep 7, 2021
961fd83
Replace type check with isinstance
pvelasco Sep 8, 2021
51d1a96
ENH: Autopopulate subjects and sessions for populate-intended-for\n\n…
pvelasco Sep 8, 2021
d53e50b
Check arguments for populate_intended_for functions only at top level
pvelasco Sep 8, 2021
0f38e5e
Merge commit 'v0.10.0-10-g80a6538' (origin/master) into adds_populate…
yarikoptic Oct 28, 2021
88e93eb
When json field is list of numbers, change evaluation to proper block
Feb 11, 2022
717f500
Merge pull request #12 from neurorepro/adds_populate_intended_for
pvelasco Feb 11, 2022
784e946
Add matching by custom label
Feb 14, 2022
45af83a
Remove unused function parameters
Feb 14, 2022
4848977
Update new function docstring to include new parameter description
Feb 14, 2022
2f6a4e9
Correct docstrings for new parameter descriptions
Feb 14, 2022
457bf40
Correct random seeding by using random library instead of numpy
Feb 15, 2022
547dafd
Set label seed for the whole test suite and print it to stdout
Feb 15, 2022
4b23544
Merge pull request #13 from neurorepro/adds_populate_intended_for
pvelasco Feb 15, 2022
97b76f3
Merge remote-tracking branch 'origin/master' into adds_populate_inten…
yarikoptic Feb 23, 2022
1c5ca68
RF+typo fix: no need for explicit list in sorted()
yarikoptic Feb 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ENH: Add BIDSFile class
... to help parse entities from BIDS filenames, as suggested in #452.

It adds one use case (in `get_key_info_for_fmap_assignment`), and the corresponding unit tests.

TO DO: RF the code using BIDSFile whenever we need to parse a BIDS filename.
pvelasco committed Jun 21, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 2886525ccff3c0592f18490b9cface731a148661
93 changes: 92 additions & 1 deletion heudiconv/bids.py
Original file line number Diff line number Diff line change
@@ -619,7 +619,7 @@ def get_key_info_for_fmap_assignment(json_file, matching_parameter='ImagingVolum
modality = op.basename(op.dirname(json_file))
if modality == 'fmap':
# extract the <acq> entity:
acq_label = re.search('(?<=[/_]acq-)\w+', json_file).group(0).split('_')[0]
acq_label = BIDSFile.parse(op.basename(json_file))['acq']
if any(s in acq_label.lower() for s in ['fmri', 'bold', 'func']):
key_info = ['func']
elif any(s in acq_label.lower() for s in ['diff', 'dwi']):
@@ -915,3 +915,94 @@ def populate_intended_for(path_to_bids_session, matching_parameters='ImagingVolu
# Add this intended_for to all fmap files in the fmap_group:
for fm_json in unique_fmap_groups[fmap_group]:
update_json(fm_json, {"IntendedFor": intended_for}, pretty=True)


class BIDSFile(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woohoo -- here it is also! ideally #544 should be finished/merged first (so would be driven by a specific copy of the schema etc), but doesn't need to be. If this PR is merged first, that one should be rebased/code moved/RFed and would immediately take advantage of this use-case use

""" as defined in https://bids-specification.readthedocs.io/en/stable/99-appendices/04-entity-table.html
which might soon become machine readable
order matters
"""

_known_entities = ['sub', 'ses', 'task', 'acq', 'ce', 'rec', 'dir', 'run', 'mod',
'echo', 'flip', 'inv', 'mt', 'part', 'recording',
]

def __init__(self, entities, suffix, extension):
self._entities = entities
self._suffix = suffix
self._extension = extension

def __eq__(self, other):
if not isinstance(other, self.__class__):
return False
if (
all([other[k] == v for k, v in self._entities.items()])
and self.extension == other.extension
and self.suffix == other.suffix
):
return True
else:
return False

@classmethod
def parse(cls, filename):
""" Parse the filename for BIDS entities, suffix and extension """
# use re.findall to find all lower-case-letters + '-' + alphanumeric + '_' pairs:
entities_list = re.findall('([a-z]+)-([a-zA-Z0-9]+)[_]*', filename)
# keep only those in the _known_entities list:
entities = {k: v for k, v in entities_list if k in BIDSFile._known_entities}
# get whatever comes after the last key-value pair, and remove any '_' that
# might come in front:
ending = filename.split('-'.join(entities_list[-1]))[-1]
ending = remove_prefix(ending, '_')
# the first dot ('.') separates the suffix from the extension:
if '.' in ending:
suffix, extension = ending.split('.', 1)
else:
suffix, extension = ending, None
return BIDSFile(entities, suffix, extension)

def __str__(self):
""" reconstitute in a legit BIDS filename using the order from entity table """
if 'sub' not in self._entities:
raise ValueError('The \'sub-\' entity is mandatory')
# reconstitute the ending for the filename:
suffix = '_' + self.suffix if self.suffix else ''
extension = '.' + self.extension if self.extension else ''
return '_'.join(
['-'.join([e, self._entities[e]]) for e in self._known_entities if e in self._entities]
) + suffix + extension

def __getitem__(self, entity):
return self._entities[entity] if entity in self._entities else None

def __setitem__(self, entity, value): # would puke with some exception if already known
return self.set(entity, value, overwrite=False)

def set(self, entity, value, overwrite=True):
if entity not in self._entities:
# just set it; no complains here
self._entities[entity] = value
elif overwrite:
lgr.warning("Overwriting the entity %s from %s to %s for file %s",
str(entity),
str(self[entity]),
str(value),
self.__str__()
)
self._entities[entity] = value
else:
# if it already exists, and overwrite is false:
lgr.warning("Setting the entity %s to %s for file %s failed",
str(entity),
str(value),
self.__str__()
)

@property # as needed make them RW
def suffix(self):
return self._suffix

@property
def extension(self):
return self._extension
69 changes: 68 additions & 1 deletion heudiconv/tests/test_bids.py
Original file line number Diff line number Diff line change
@@ -4,7 +4,9 @@
import re
import os
import os.path as op
from random import random
from random import (random,
shuffle,
)
from datetime import (datetime,
timedelta,
)
@@ -33,6 +35,7 @@
SHIM_KEY,
AllowedCriteriaForFmapAssignment,
KeyInfoForForce,
BIDSFile,
)

import pytest
@@ -868,3 +871,67 @@ def test_populate_intended_for(
assert '{p}_acq-unmatched_bold.nii.gz'.format(p=run_prefix) not in data['IntendedFor']
else:
assert 'IntendedFor' not in data.keys()


def test_BIDSFile():
""" Tests for the BIDSFile class """

# define entities in the correct order:
sorted_entities = [
('sub', 'Jason'),
('acq', 'Treadstone'),
('run', '2'),
('echo', '1'),
]
# 'sub-Jason_acq-Treadstone_run-2_echo-1':
expected_sorted_str = '_'.join(['-'.join(e) for e in sorted_entities])
# expected BIDSFile:
suffix = 'T1w'
extension = 'nii.gz'
expected_bids_file = BIDSFile(OrderedDict(sorted_entities), suffix, extension)

# entities in random order:
idcs = list(range(len(sorted_entities)))
shuffle(idcs)
shuffled_entities = [sorted_entities[i] for i in idcs]
shuffled_str = '_'.join(['-'.join(e) for e in shuffled_entities])

# Test __eq__ method.
# It should consider equal BIDSFiles with the same entities even in different order:
assert BIDSFile(OrderedDict(shuffled_entities), suffix, extension) == expected_bids_file

# Test __getitem__:
assert all([expected_bids_file[k] == v for k, v in shuffled_entities])

# Test filename parser and __str__ method:
# Note: the __str__ method should return entities in the correct order
ending = '_T1w.nii.gz' # suffix + extension
my_bids_file = BIDSFile.parse(shuffled_str + ending)
assert my_bids_file == expected_bids_file
assert str(my_bids_file) == expected_sorted_str + ending

ending = '.json' # just extension
my_bids_file = BIDSFile.parse(shuffled_str + ending)
assert my_bids_file.suffix == ''
assert str(my_bids_file) == expected_sorted_str + ending

ending = '_T1w' # just suffix
my_bids_file = BIDSFile.parse(shuffled_str + ending)
assert my_bids_file.extension is None
assert str(my_bids_file) == expected_sorted_str + ending

# Complain if entity 'sub' is not set:
with pytest.raises(ValueError) as e_info:
assert str(BIDSFile.parse('dir-reversed.json'))
assert 'sub-' in e_info.value

# Test set method:
# -for a new entity, just set it without a complaint:
my_bids_file['dir'] = 'AP'
assert my_bids_file['dir'] == 'AP'
# -for an existing entity, don't change it by default:
my_bids_file['echo'] = '2'
assert my_bids_file['echo'] == expected_bids_file['echo'] # still the original value
# -for an existing entity, you can overwrite it with "set":
my_bids_file.set('echo', '2')
assert my_bids_file['echo'] == '2'