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

Rename s variable to curr_seqinfo in reproin heuristic #779

Merged
merged 1 commit into from
Aug 8, 2024
Merged
Changes from all commits
Commits
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
77 changes: 41 additions & 36 deletions heudiconv/heuristics/reproin.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@
"""Function that adds cancelme_ to known bad runs which were forgotten"""
if not fix_accession2run:
return seqinfo # nothing to do
for i, s in enumerate(seqinfo):
accession_number = s.accession_number
for i, curr_seqinfo in enumerate(seqinfo):
accession_number = curr_seqinfo.accession_number
if accession_number and accession_number in fix_accession2run:
lgr.info(
"Considering some runs possibly marked to be "
Expand All @@ -292,12 +292,12 @@
# a single accession, but left as is for now
badruns = fix_accession2run[accession_number]
badruns_pattern = "|".join(badruns)
if re.match(badruns_pattern, s.series_id):
lgr.info("Fixing bad run {0}".format(s.series_id))
if re.match(badruns_pattern, curr_seqinfo.series_id):
lgr.info("Fixing bad run {0}".format(curr_seqinfo.series_id))
fixedkwargs = dict()
for key in series_spec_fields:
fixedkwargs[key] = "cancelme_" + getattr(s, key)
seqinfo[i] = s._replace(**fixedkwargs)
fixedkwargs[key] = "cancelme_" + getattr(curr_seqinfo, key)
seqinfo[i] = curr_seqinfo._replace(**fixedkwargs)
return seqinfo


Expand Down Expand Up @@ -341,19 +341,19 @@
seqinfo: list[SeqInfo], substitutions: list[tuple[str, str]], subs_scope: str
) -> None:
lgr.info("Considering %s substitutions", subs_scope)
for i, s in enumerate(seqinfo):
for i, curr_seqinfo in enumerate(seqinfo):
fixed_kwargs = dict()
# need to replace both protocol_name series_description
for key in series_spec_fields:
oldvalue = value = getattr(s, key)
oldvalue = value = getattr(curr_seqinfo, key)
# replace all I need to replace
for substring, replacement in substitutions:
value = re.sub(substring, replacement, value)
if oldvalue != value:
lgr.info(" %s: %r -> %r", key, oldvalue, value)
fixed_kwargs[key] = value
# namedtuples are immutable
seqinfo[i] = s._replace(**fixed_kwargs)
seqinfo[i] = curr_seqinfo._replace(**fixed_kwargs)


def fix_seqinfo(seqinfo: list[SeqInfo]) -> list[SeqInfo]:
Expand Down Expand Up @@ -402,32 +402,34 @@
run_label: Optional[str] = None # run-
dcm_image_iod_spec: Optional[str] = None
skip_derived = False
for s in seqinfo:
for curr_seqinfo in seqinfo:
# XXX: skip derived sequences, we don't store them to avoid polluting
# the directory, unless it is the motion corrected ones
# (will get _rec-moco suffix)
if skip_derived and s.is_derived and not s.is_motion_corrected:
skipped.append(s.series_id)
lgr.debug("Ignoring derived data %s", s.series_id)
if skip_derived and curr_seqinfo.is_derived and not curr_seqinfo.is_motion_corrected:
skipped.append(curr_seqinfo.series_id)
lgr.debug("Ignoring derived data %s", curr_seqinfo.series_id)

Check warning on line 411 in heudiconv/heuristics/reproin.py

View check run for this annotation

Codecov / codecov/patch

heudiconv/heuristics/reproin.py#L410-L411

Added lines #L410 - L411 were not covered by tests
continue

# possibly apply present formatting in the series_description or protocol name
for f in "series_description", "protocol_name":
s = s._replace(**{f: getattr(s, f).format(**s._asdict())})
curr_seqinfo = curr_seqinfo._replace(
**{f: getattr(curr_seqinfo, f).format(**curr_seqinfo._asdict())}
)

template = None
suffix = ""
# seq = []

# figure out type of image from s.image_info -- just for checking ATM
# figure out type of image from curr_seqinfo.image_info -- just for checking ATM
# since we primarily rely on encoded in the protocol name information
prev_dcm_image_iod_spec = dcm_image_iod_spec
if len(s.image_type) > 2:
if len(curr_seqinfo.image_type) > 2:
# https://dicom.innolitics.com/ciods/cr-image/general-image/00080008
# 0 - ORIGINAL/DERIVED
# 1 - PRIMARY/SECONDARY
# 3 - Image IOD specific specialization (optional)
dcm_image_iod_spec = s.image_type[2]
dcm_image_iod_spec = curr_seqinfo.image_type[2]
image_type_datatype = {
# Note: P and M are too generic to make a decision here, could be
# for different datatypes (bold, fmap, etc)
Expand All @@ -443,7 +445,7 @@

series_info = {} # For please lintian and its friends
for sfield in series_spec_fields:
svalue = getattr(s, sfield)
svalue = getattr(curr_seqinfo, sfield)
series_info = parse_series_spec(svalue)
if series_info: # looks like a valid spec - we are done
series_spec = svalue
Expand All @@ -454,10 +456,10 @@
if not series_info:
series_spec = None # we cannot know better
lgr.warning(
"Could not determine the series name by looking at " "%s fields",
"Could not determine the series name by looking at %s fields",
", ".join(series_spec_fields),
)
skipped_unknown.append(s.series_id)
skipped_unknown.append(curr_seqinfo.series_id)
continue

if dcm_image_iod_spec and dcm_image_iod_spec.startswith("MIP"):
Expand All @@ -476,14 +478,14 @@
series_spec,
)

# if s.is_derived:
# if curr_seqinfo.is_derived:
# # Let's for now stash those close to original images
# # TODO: we might want a separate tree for all of this!?
# # so more of a parameter to the create_key
# #datatype += '/derivative'
# # just keep it lower case and without special characters
# # XXXX what for???
# #seq.append(s.series_description.lower())
# #seq.append(curr_seqinfo.series_description.lower())
# prefix = os.path.join('derivatives', 'scanner')
# else:
# prefix = ''
Expand All @@ -493,14 +495,14 @@
# Figure out the datatype_suffix (BIDS _suffix)
#
# If none was provided -- let's deduce it from the information we find:
# analyze s.protocol_name (series_id is based on it) for full name mapping etc
# analyze curr_seqinfo.protocol_name (series_id is based on it) for full name mapping etc
if not datatype_suffix:
if datatype == "func":
if "_pace_" in series_spec:
datatype_suffix = "pace" # or should it be part of seq-
elif "P" in s.image_type:
elif "P" in curr_seqinfo.image_type:
datatype_suffix = "phase"
elif "M" in s.image_type:
elif "M" in curr_seqinfo.image_type:
datatype_suffix = "bold"
else:
# assume bold by default
Expand All @@ -526,7 +528,7 @@
# since they are complementary files produced along-side with original
# ones.
#
if s.series_description.endswith("_SBRef"):
if curr_seqinfo.series_description.endswith("_SBRef"):
datatype_suffix = "sbref"

if not datatype_suffix:
Expand All @@ -550,7 +552,7 @@
# XXX if we have a known earlier study, we need to always
# increase the run counter for phasediff because magnitudes
# were not acquired
if get_study_hash([s]) == "9d148e2a05f782273f6343507733309d":
if get_study_hash([curr_seqinfo]) == "9d148e2a05f782273f6343507733309d":

Check warning on line 555 in heudiconv/heuristics/reproin.py

View check run for this annotation

Codecov / codecov/patch

heudiconv/heuristics/reproin.py#L555

Added line #L555 was not covered by tests
current_run += 1
else:
raise RuntimeError(
Expand Down Expand Up @@ -583,10 +585,10 @@
run_label = None

# yoh: had a wrong assumption
# if s.is_motion_corrected:
# assert s.is_derived, "Motion corrected images must be 'derived'"
# if curr_seqinfo.is_motion_corrected:
# assert curr_seqinfo.is_derived, "Motion corrected images must be 'derived'"

if s.is_motion_corrected and "rec-" in series_info.get("bids", ""):
if curr_seqinfo.is_motion_corrected and "rec-" in series_info.get("bids", ""):
raise NotImplementedError(
"want to add _rec-moco but there is _rec- already"
)
Expand All @@ -611,7 +613,7 @@
from_series_info("acq"),
# But we want to add an indicator in case it was motion corrected
# in the magnet. ref sample /2017/01/03/qa
None if not s.is_motion_corrected else "rec-moco",
None if not curr_seqinfo.is_motion_corrected else "rec-moco",
from_series_info("dir"),
series_info.get("bids"),
run_label,
Expand All @@ -621,7 +623,7 @@
suffix = "_".join(filter(bool, filename_suffix_parts)) # type: ignore[arg-type]

# # .series_description in case of
# sdesc = s.study_description
# sdesc = curr_seqinfo.study_description
# # temporary aliases for those phantoms which we already collected
# # so we rename them into this
# #MAPPING
Expand All @@ -638,13 +640,16 @@
# https://github.com/nipy/heudiconv/issues/145
outtype: tuple[str, ...]
if (
"_Scout" in s.series_description
"_Scout" in curr_seqinfo.series_description
or (
datatype == "anat"
and datatype_suffix
and datatype_suffix.startswith("scout")
)
or (s.series_description.lower() == s.protocol_name.lower() + "_setter")
or (
curr_seqinfo.series_description.lower()
== curr_seqinfo.protocol_name.lower() + "_setter"
)
):
outtype = ("dicom",)
else:
Expand All @@ -654,7 +659,7 @@
# we wanted ordered dict for consistent demarcation of dups
if template not in info:
info[template] = []
info[template].append(s.series_id)
info[template].append(curr_seqinfo.series_id)

if skipped:
lgr.info("Skipped %d sequences: %s" % (len(skipped), skipped))
Expand Down Expand Up @@ -762,7 +767,7 @@

# So -- use `outdir` and locator etc to see if for a given locator/subject
# and possible ses+ in the sequence names, so we would provide a sequence
# So might need to go through parse_series_spec(s.protocol_name)
# So might need to go through parse_series_spec(curr_seqinfo.protocol_name)
# to figure out presence of sessions.
ses_markers: list[str] = []

Expand Down