🎨 Run pre-commit hooks on whole repo

[shnizzedy]

Fixes

Related to #2048 by @nx10

Description

• Moves the syntax checkers before the language formatters in .pre-commit-config.yaml
• Disables pretty-format-yaml because that formatters doesn't accept / support directives, and many of the YAML documents in this repo use the YAML directive
• Runs

  ruff . --fix --unsafe-fixes
  pre-commit run --all-files

  as @ChildMindInstituteCNL
• ✅ Fix tests broken by #2058 #2059
  • 🎨 print statements → log statements #2062
  • 🎨 PEP8 boolean comparisons #2063
  • ⚰️ Remove dead code (create_fsl_model) #2078
  • 🥅 Raise an error instead of returning None where a dict is expected #2079
  • 🐛 For uploading logs, look for S3 encryption setting in pipeline config #2080
• Updated links to master branches to main (if same repo) or permalinks (if other repo) in comments and docstrings (82aab12)
• Where I noticed code from other codebases without license notices, I injected license notices (5e66a2b‒bb5cef4, 82aab12‒f667e95, 8720fe8‒a2da0a4, 77aad8d, 903b06d-72b50a4, a52125b)

Technical details

The errors listed here are unresolved in this PR. I'm inclined to use this strategy for those errors ― the next time someone modifies one of those files, they can fix the errors in that file, either in the same PR or in a preceding PR.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the ruff branch of the repository.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added tests for the changes I made (if applicable).
• I updated the changelog.
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[shnizzedy]

I noticed this nodeblock uses the input "motion-basefile" but doesn't use the input "desc-preproc_bold" even though it's listed in the inputs in the decorator here.

[shnizzedy]

(To consider later): The original codebase has a MatchHeader interface that we could use instead of this chain of nodes

[shnizzedy]

1. s_param_map isn't used anywhere?

2. C-PAC/CPAC/utils/extract_data.py

   Lines 40 to 476 in 95074d2

   	def extract_data(c, param_map):
   	"""
   	Generate a CPAC input subject list Python file.
   	The method extracts anatomical and functional data for each site (if multiple site)
   	and/or scan parameters for each site and put it into a data structure read by Python.
   	Examples
   	--------
   	subjects_list =[
   	{
   	'subject_id' : '0050386',
   	'unique_id' : 'session_1',
   	'anat': '/Users/home/data/NYU/0050386/session_1/anat_1/anat.nii.gz',
   	'rest':{
   	'rest_1_rest' : '/Users/home/data/NYU/0050386/session_1/rest_1/rest.nii.gz',
   	'rest_2_rest' : '/Users/home/data/NYU/0050386/session_1/rest_2/rest.nii.gz',
   	}
   	'scan_parameters':{
   	'tr': '2',
   	'acquisition': 'alt+z2',
   	'reference': '17',
   	'first_tr': '',
   	'last_tr': '',
   	}
   	},
   	]
   	or
   	subjects_list =[
   	{
   	'subject_id' : '0050386',
   	'unique_id' : 'session_1',
   	'anat': '/Users/home/data/NYU/0050386/session_1/anat_1/anat.nii.gz',
   	'rest':{
   	'rest_1_rest' : '/Users/home/data/NYU/0050386/session_1/rest_1/rest.nii.gz',
   	'rest_2_rest' : '/Users/home/data/NYU/0050386/session_1/rest_2/rest.nii.gz',
   	}
   	},
   	]
   	"""
   	def get_list(arg) -> list:
   	"""Read each line of the file into list."""
   	if isinstance(arg, list):
   	ret_list = arg
   	else:
   	ret_list = [fline.rstrip("\r\n") for fline in open(arg, "r").readlines()]
   	return ret_list
   	exclusion_list = []
   	if c.exclusionSubjectList is not None:
   	exclusion_list = get_list(c.exclusionSubjectList)
   	subject_list = []
   	if c.subjectList is not None:
   	subject_list = get_list(c.subjectList)
   	def checkTemplate(template) -> None:
   	"""Check if `template` is correct."""
   	if template.count("%s") != 2:
   	msg = (
   	"Please provide '%s' in the template"
   	"where your site and subjects are present"
   	"Please see examples"
   	)
   	logger.exception(msg)
   	raise Exception(msg)
   	filename, ext = os.path.splitext(os.path.basename(template))
   	ext = os.path.splitext(filename)[1] + ext
   	if ext not in [".nii", ".nii.gz"]:
   	msg = "Invalid file name", os.path.basename(template)
   	logger.exception(msg)
   	raise Exception(msg)
   	def get_site_list(path):
   	base, relative = path.split("%s")
   	return os.listdir(base)
   	def check_length(scan_name, file_name):
   	if len(file_name) > 30:
   	msg = (
   	"filename- %s is too long."
   	"It should not be more than 30 characters." % (file_name)
   	)
   	logger.exception(msg)
   	raise Exception(msg)
   	if (
   	len(scan_name) - len(os.path.splitext(os.path.splitext(file_name)[0])[0])
   	>= 40
   	):
   	msg = (
   	"scan name %s is too long."
   	"It should not be more than 20 characters"
   	% (
   	scan_name.replace(
   	"_" + os.path.splitext(os.path.splitext(file_name)[0])[0], ""
   	)
   	)
   	)
   	logger.exception(msg)
   	raise Exception(msg)
   	def create_site_subject_mapping(base, relative):
   	"""Create mapping between site and subject."""
   	site_subject_map = {}
   	base_path_list = []
   	if c.siteList is not None:
   	site_list = get_list(c.siteList)
   	else:
   	site_list = get_site_list(base)
   	for site in site_list:
   	paths = glob.glob(string.replace(base, "%s", site))
   	base_path_list.extend(paths)
   	for path in paths:
   	for sub in os.listdir(path):
   	# check if subject is present in subject_list
   	if subject_list:
   	if sub in subject_list and sub not in exclusion_list:
   	site_subject_map[sub] = site
   	elif sub not in exclusion_list:
   	if sub not in ".DS_Store":
   	site_subject_map[sub] = site
   	return base_path_list, site_subject_map
   	def getPath(template):
   	"""Split the input template path...
   	...into base, path before subject directory and relative, path after subject directory.
   	"""
   	checkTemplate(template)
   	base, relative = template.rsplit("%s", 1)
   	base, subject_map = create_site_subject_mapping(base, relative)
   	base.sort()
   	relative = relative.lstrip("/")
   	return base, relative, subject_map
   	# get anatomical base path and anatomical relative path
   	anat_base, anat_relative = getPath(c.anatomicalTemplate)[:2]
   	# get functional base path, functional relative path and site-subject map
   	func_base, func_relative, subject_map = getPath(c.functionalTemplate)
   	if not anat_base:
   	msg = (
   	"Anatomical Data template incorrect. No such file or directory %s",
   	anat_base,
   	)
   	logger.exception(msg)
   	raise Exception(msg)
   	if not func_base:
   	msg = "Functional Data template incorrect. No such file or directory %s, func_base"
   	logger.exception(msg)
   	raise Exception(msg)
   	if len(anat_base) != len(func_base):
   	msg1 = (
   	"Some sites are missing, Please check your template",
   	anat_base,
   	"!=",
   	func_base,
   	)
   	logger.exception(msg1)
   	msg2 = (
   	" Base length Unequal. Some sites are missing."
   	"extract_data doesn't script support this.Please"
   	"Provide your own subjects_list file"
   	)
   	logger.exception(msg2)
   	raise Exception(msg2)
   	# calculate the length of relative paths(path after subject directory)
   	func_relative_len = len(func_relative.split("/"))
   	anat_relative_len = len(anat_relative.split("/"))
   	def check_for_sessions(relative_path, path_length):
   	"""Check if there are sessions present."""
   	# default
   	session_present = False
   	session_path = "session_1"
   	# session present if path_length is equal to 3
   	if path_length == 3: # noqa: PLR2004
   	relative_path_list = relative_path.split("/")
   	session_path = relative_path_list[0]
   	relative_path = string.join(relative_path_list[1:], "/")
   	session_present = True
   	elif path_length > 3: # noqa: PLR2004
   	msg = (
   	"extract_data script currently doesn't support this directory structure."
   	"Please provide the subjects_list file to run CPAC."
   	"For more information refer to manual"
   	)
   	logger.exception(msg)
   	raise Exception(msg)
   	return session_present, session_path, relative_path
   	func_session_present, func_session_path, func_relative = check_for_sessions(
   	func_relative, func_relative_len
   	)
   	anat_session_present, anat_session_path, anat_relative = check_for_sessions(
   	anat_relative, anat_relative_len
   	)
   	f = open(
   	os.path.join(
   	c.outputSubjectListLocation, "CPAC_subject_list_%s.yml" % c.subjectListName
   	),
   	"wb",
   	)
   	def fetch_path(i, anat_sub, func_sub, session_id):
   	"""
   	Extract anatomical and functional path for a session and print to file.
   	Parameters
   	----------
   	i : int
   	index of site
   	anat_sub : string
   	string containing subject/ concatenated
   	subject-session path for anatomical file
   	func_sub : string
   	string containing subject/ concatenated
   	subject-session path for functional file
   	session_id : string
   	session
   	Raises
   	------
   	Exception
   	"""
   	try:
   	def print_begin_of_file(sub, session_id):
   	print("-", file=f)
   	print(" subject_id: '" + sub + "'", file=f)
   	print(" unique_id: '" + session_id + "'", file=f)
   	def print_end_of_file(sub):
   	if param_map is not None:
   	try:
   	logger.debug("site for sub %s -> %s", sub, subject_map.get(sub))
   	logger.debug(
   	"scan parameters for the above site %s",
   	param_map.get(subject_map.get(sub)),
   	)
   	print(" scan_parameters:", file=f)
   	print(
   	" tr: '"
   	+ param_map.get(subject_map.get(sub))[4]
   	+ "'",
   	file=f,
   	)
   	print(
   	" acquisition: '"
   	+ param_map.get(subject_map.get(sub))[0]
   	+ "'",
   	file=f,
   	)
   	print(
   	" reference: '"
   	+ param_map.get(subject_map.get(sub))[3]
   	+ "'",
   	file=f,
   	)
   	print(
   	" first_tr: '"
   	+ param_map.get(subject_map.get(sub))[1]
   	+ "'",
   	file=f,
   	)
   	print(
   	" last_tr: '"
   	+ param_map.get(subject_map.get(sub))[2]
   	+ "'",
   	file=f,
   	)
   	except:
   	msg = (
   	" No Parameter values for the %s site is defined in the scan"
   	" parameters csv file" % subject_map.get(sub)
   	)
   	raise ValueError(msg)
   	# get anatomical file
   	anat_base_path = os.path.join(anat_base[i], anat_sub)
   	func_base_path = os.path.join(func_base[i], func_sub)
   	anat = None
   	func = None
   	anat = glob.glob(os.path.join(anat_base_path, anat_relative))
   	func = glob.glob(os.path.join(func_base_path, func_relative))
   	if anat and func:
   	print_begin_of_file(anat_sub.split("/")[0], session_id)
   	print(" anat: '" + os.path.realpath(anat[0]) + "'", file=f)
   	print(" rest: ", file=f)
   	# iterate for each rest session
   	for _iter in func:
   	# get scan_id
   	iterable = os.path.splitext(
   	os.path.splitext(_iter.replace(func_base_path, "").lstrip("/"))[
   	0
   	]
   	)[0]
   	iterable = iterable.replace("/", "_")
   	check_length(iterable, os.path.basename(os.path.realpath(_iter)))
   	print(
   	" " + iterable + ": '" + os.path.realpath(_iter) + "'",
   	file=f,
   	)
   	print_end_of_file(anat_sub.split("/")[0])
   	else:
   	logger.debug("skipping subject %s", anat_sub.split("/")[0])
   	except ValueError:
   	logger.exception(ValueError.message)
   	raise
   	except Exception as e:
   	err_msg = (
   	"Exception while felching anatomical and functional "
   	"paths: \n" + str(e)
   	)
   	logger.exception(err_msg)
   	raise Exception(err_msg)
   	def walk(index, sub):
   	"""
   	Walk across each subject path in the data site path.
   	Parameters
   	----------
   	index : int
   	index of site
   	sub : string
   	subject_id
   	Raises
   	------
   	Exception
   	"""
   	try:
   	if func_session_present:
   	# if there are sessions
   	if "*" in func_session_path:
   	session_list = glob.glob(
   	os.path.join(
   	func_base[index], os.path.join(sub, func_session_path)
   	)
   	)
   	else:
   	session_list = [func_session_path]
   	if session_list:
   	for session in session_list:
   	session_id = os.path.basename(session)
   	if anat_session_present:
   	if func_session_path == anat_session_path:
   	fetch_path(
   	index,
   	os.path.join(sub, session_id),
   	os.path.join(sub, session_id),
   	session_id,
   	)
   	else:
   	fetch_path(
   	index,
   	os.path.join(sub, anat_session_path),
   	os.path.join(sub, session_id),
   	session_id,
   	)
   	else:
   	fetch_path(
   	index, sub, os.path.join(sub, session_id), session_id
   	)
   	else:
   	logger.debug("Skipping subject %s", sub)
   	else:
   	logger.debug("No sessions")
   	session_id = ""
   	fetch_path(index, sub, sub, session_id)
   	except Exception:
   	logger.exception(Exception.message)
   	raise
   	except:
   	err_msg = "Please make sessions are consistent across all subjects.\n\n"
   	logger.exception(err_msg)
   	raise Exception(err_msg)
   	try:
   	for i in range(len(anat_base)):
   	for sub in os.listdir(anat_base[i]):
   	# check if subject is present in subject_list
   	if subject_list:
   	if sub in subject_list and sub not in exclusion_list:
   	logger.debug("extracting data for subject: %s", sub)
   	walk(i, sub)
   	# check that subject is not in exclusion list
   	elif sub not in exclusion_list and sub not in ".DS_Store":
   	logger.debug("extracting data for subject: %s", sub)
   	walk(i, sub)
   	_name = os.path.join(c.outputSubjectListLocation, "CPAC_subject_list.yml")
   	logger.info(
   	"Extraction Successfully Completed...Input Subjects_list for CPAC - %s",
   	_name,
   	)
   	except Exception:
   	logger.exception(Exception.message)
   	raise
   	finally:
   	f.close()

   isn't called anywhere?
3. CPAC/utils/extract_data.py file and CPAC/utils/extract_data_multiscan.py have a lot of WET overlap? In extract_data_multiscan,

   C-PAC/CPAC/utils/extract_data_multiscan.py

   Line 565 in 95074d2

   extract_data(c, s_param_map)

4. Neither extract_data nor extract_data_multiscan is imported or used anywhere as far as I can tell?

Given all this, I think we can delete both of these files?

[shnizzedy]

The import for warp_denoiseNofilt_to_T1template was removed in a8782f2 / #1889 (diff).

Do we want it gone and can remove this, or should we restore the import?

[shnizzedy]

The output for the resource "space-fsLR_den-32k_desc-FS_strainJ" is set to "StrainJ_FS_dscalar" and then reset to "StrainJ_MSMSulc_dscalar" three lines later.

I think the second one is supposed to be "space-fsLR_den-32k_desc-MSMSulc_strainJ" based on the pattern of changes in the commit in which this was set up (d6a9eab#diff-dcbc5442d7f74e5b292b813146582f950ca1dc1c7b96693c2dbde66e53105db8R719-R722).

I don't see either of these resources used anywhere, so it probably isn't causing any problems.

[shnizzedy]

We've got two definitions for compute_fisher_z_score (this and

C-PAC/CPAC/utils/utils.py

Lines 429 to 472 in 2cc59aa

	def compute_fisher_z_score(correlation_file, timeseries_one_d, input_name):
	"""Compute the fisher z transform of the input correlation map.
	If the correlation map contains data for multiple ROIs then
	return z score for each ROI as a seperate NIfTI file.
	Parameters
	----------
	correlation_file : string
	Input correlations file
	Returns
	-------
	out_file : list (nifti files)
	list of z_scores for mask or ROI
	"""
	import os
	import numpy as np
	import nibabel as nib
	# get the specific roi number
	filename = correlation_file.split("/")[-1]
	filename = filename.replace(".nii", "")
	if ".gz" in filename:
	filename = filename.replace(".gz", "")
	corr_img = nib.load(correlation_file)
	corr_data = corr_img.get_fdata()
	hdr = corr_img.header
	# calculate the Fisher r-to-z transformation
	corr_data = np.log((1 + corr_data) / (1 - corr_data)) / 2.0
	z_score_img = nib.Nifti1Image(corr_data, header=hdr, affine=corr_img.affine)
	out_file = os.path.join(os.getcwd(), filename + "_fisher_zstd.nii.gz")
	z_score_img.to_filename(out_file)
	return out_file

). That one doesn't use the timeseries file at all; this one isn't called anywhere.

[shnizzedy]

We've got two definitions for compute_fisher_z_score (this and

C-PAC/CPAC/sca/utils.py

Lines 22 to 91 in 2cc59aa

	def compute_fisher_z_score(correlation_file, timeseries_one_d):
	"""
	Computes the fisher z transform of the input correlation map
	If the correlation map contains data for multiple ROIs then
	the function returns z score for each ROI as a seperate nifti
	file.
	Parameters
	----------
	correlation_file : string
	Input correlations file
	Returns
	-------
	out_file : list (nifti files)
	list of z_scores for mask or ROI
	"""
	import os
	import numpy as np
	import nibabel as nib
	roi_numbers = []
	if "#" in open(timeseries_one_d, "r").readline().rstrip("\r\n"):
	roi_numbers = (
	open(timeseries_one_d, "r")
	.readline()
	.rstrip("\r\n")
	.replace("#", "")
	.split("\t")
	)
	corr_img = nib.load(correlation_file)
	corr_data = corr_img.get_fdata()
	hdr = corr_img.header
	corr_data = np.log((1 + corr_data) / (1 - corr_data)) / 2.0
	dims = corr_data.shape
	out_file = []
	if len(dims) == 5 or len(roi_numbers) > 0:
	if len(dims) == 5:
	x, y, z, one, roi_number = dims
	corr_data = np.reshape(corr_data, (x * y * z, roi_number), order="F")
	for i in range(0, len(roi_numbers)):
	sub_data = corr_data
	if len(dims) == 5:
	sub_data = np.reshape(corr_data[:, i], (x, y, z), order="F")
	sub_img = nib.Nifti1Image(
	sub_data, header=corr_img.header, affine=corr_img.affine
	)
	sub_z_score_file = os.path.join(
	os.getcwd(), "z_score_ROI_number_%s.nii.gz" % (roi_numbers[i])
	)
	sub_img.to_filename(sub_z_score_file)
	out_file.append(sub_z_score_file)
	else:
	z_score_img = nib.Nifti1Image(corr_data, header=hdr, affine=corr_img.affine)
	z_score_file = os.path.join(os.getcwd(), "z_score.nii.gz")
	z_score_img.to_filename(z_score_file)
	out_file.append(z_score_file)
	return out_file

). That one isn't called anywhere; this one doesn't use the timeseries file at all.