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

STY: Lint and style check full repository #3221

Merged
merged 7 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
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
2 changes: 2 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# 2024-02-05 - [email protected] - STY: ruff --fix + 1 ignore [git-blame-ignore-rev]
e7dc59fbc7df88dfabbff154f4cc3d24721b6b4f
# 2024-01-16 - [email protected] - STY: ruff --fix --unsafe-fixes (with cleanup) [git-blame-ignore-rev]
66734bd0164d1dae3cf299fa9d9d682c22eeda66
# 2024-01-16 - [email protected] - STY: ruff format and ruff --fix [git-blame-ignore-rev]
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/contrib.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: pipx run ruff fmriprep
- run: pipx run ruff format --diff fmriprep
- run: pipx run ruff .
- run: pipx run ruff format --diff .

codespell:
name: Check for spelling errors
Expand Down
Empty file modified .maint/paper_author_list.py
100644 → 100755
Empty file.
Empty file modified .maint/update_authors.py
100644 → 100755
Empty file.
4 changes: 2 additions & 2 deletions fmriprep/cli/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,26 @@
fmriprep_dir = config.execution.fmriprep_dir
version = config.environment.version

retval['return_code'] = 1
retval['workflow'] = None

Check warning on line 55 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L54-L55

Added lines #L54 - L55 were not covered by tests

banner = [f'Running fMRIPrep version {version}']
notice_path = data.load.readable('NOTICE')

Check warning on line 58 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L57-L58

Added lines #L57 - L58 were not covered by tests
if notice_path.exists():
banner[0] += '\n'

Check warning on line 60 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L60

Added line #L60 was not covered by tests
banner += [f"License NOTICE {'#' * 50}"]
banner += [f'fMRIPrep {version}']

Check warning on line 62 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L62

Added line #L62 was not covered by tests
banner += notice_path.read_text().splitlines(keepends=False)[1:]
banner += ['#' * len(banner[1])]

Check warning on line 64 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L64

Added line #L64 was not covered by tests
build_log.log(25, f"\n{' ' * 9}".join(banner))

# warn if older results exist: check for dataset_description.json in output folder
msg = check_pipeline_version('fMRIPrep', version, fmriprep_dir / 'dataset_description.json')

Check warning on line 68 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L68

Added line #L68 was not covered by tests
if msg is not None:
build_log.warning(msg)

# Please note this is the input folder's dataset_description.json
dset_desc_path = config.execution.bids_dir / 'dataset_description.json'

Check warning on line 73 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L73

Added line #L73 was not covered by tests
if dset_desc_path.exists():
from hashlib import sha256

Expand All @@ -84,7 +84,7 @@

# Called with reports only
if config.execution.reports_only:
build_log.log(25, 'Running --reports-only on participants %s', ', '.join(subject_list))

Check warning on line 87 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L87

Added line #L87 was not covered by tests
session_list = (
config.execution.bids_filters.get('bold', {}).get('session')
if config.execution.bids_filters
Expand All @@ -103,7 +103,7 @@
', '.join(failed_reports),
)

retval['return_code'] = len(failed_reports)

Check warning on line 106 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L106

Added line #L106 was not covered by tests
return retval

# Build main workflow
Expand All @@ -116,14 +116,14 @@
]

if config.execution.derivatives:
init_msg += [f'Searching for derivatives: {config.execution.derivatives}.']

Check warning on line 119 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L119

Added line #L119 was not covered by tests

if config.execution.fs_subjects_dir:
init_msg += [f"Pre-run FreeSurfer's SUBJECTS_DIR: {config.execution.fs_subjects_dir}."]

build_log.log(25, f"\n{' ' * 11}* ".join(init_msg))

retval['workflow'] = init_fmriprep_wf()

Check warning on line 126 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L126

Added line #L126 was not covered by tests

# Check for FS license after building the workflow
if not check_valid_fs_license():
Expand All @@ -144,17 +144,17 @@
2) ``$FS_LICENSE`` environment variable; and 3) the ``$FREESURFER_HOME/license.txt`` path. Get it \
(for free) by registering at https://surfer.nmr.mgh.harvard.edu/registration.html"""
)
retval['return_code'] = 126 # 126 == Command invoked cannot execute.

Check warning on line 147 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L147

Added line #L147 was not covered by tests
return retval

# Check workflow for missing commands
missing = check_deps(retval['workflow'])

Check warning on line 151 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L151

Added line #L151 was not covered by tests
if missing:
build_log.critical(
'Cannot run fMRIPrep. Missing dependencies:%s',
'\n\t* '.join([''] + [f'{cmd} (Interface: {iface})' for iface, cmd in missing]),
)
retval['return_code'] = 127 # 127 == command not found.

Check warning on line 157 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L157

Added line #L157 was not covered by tests
return retval

config.to_filename(config_file)
Expand All @@ -162,7 +162,7 @@
'fMRIPrep workflow graph with %d nodes built successfully.',
len(retval['workflow']._get_all_nodes()),
)
retval['return_code'] = 0

Check warning on line 165 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L165

Added line #L165 was not covered by tests
return retval


Expand All @@ -171,7 +171,7 @@
from .. import config

config.load(config_file)
logs_path = config.execution.fmriprep_dir / 'logs'

Check warning on line 174 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L174

Added line #L174 was not covered by tests
boilerplate = workflow.visit_desc()
citation_files = {
ext: logs_path / ('CITATION.%s' % ext) for ext in ('bib', 'tex', 'md', 'html')
Expand All @@ -187,15 +187,15 @@
except FileNotFoundError:
pass

citation_files['md'].write_text(boilerplate)

Check warning on line 190 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L190

Added line #L190 was not covered by tests

if not config.execution.md_only_boilerplate and citation_files['md'].exists():

Check warning on line 192 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L192

Added line #L192 was not covered by tests
from subprocess import CalledProcessError, TimeoutExpired, check_call

from .. import data

bib_text = data.load.readable('boilerplate.bib').read_text()
citation_files['bib'].write_text(

Check warning on line 198 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L197-L198

Added lines #L197 - L198 were not covered by tests
bib_text.replace('fMRIPrep <version>', f'fMRIPrep {config.environment.version}')
)

Expand All @@ -213,11 +213,11 @@
str(citation_files['html']),
]

config.loggers.cli.info('Generating an HTML version of the citation boilerplate...')

Check warning on line 216 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L216

Added line #L216 was not covered by tests
try:
check_call(cmd, timeout=10) # noqa: S603
check_call(cmd, timeout=10)
except (FileNotFoundError, CalledProcessError, TimeoutExpired):
config.loggers.cli.warning('Could not generate CITATION.html file:\n%s', ' '.join(cmd))

Check warning on line 220 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L220

Added line #L220 was not covered by tests

# Generate LaTex file resolving citations
cmd = [
Expand All @@ -230,8 +230,8 @@
'-o',
str(citation_files['tex']),
]
config.loggers.cli.info('Generating a LaTeX version of the citation boilerplate...')

Check warning on line 233 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L233

Added line #L233 was not covered by tests
try:
check_call(cmd, timeout=10) # noqa: S603
check_call(cmd, timeout=10)
except (FileNotFoundError, CalledProcessError, TimeoutExpired):
config.loggers.cli.warning('Could not generate CITATION.tex file:\n%s', ' '.join(cmd))

Check warning on line 237 in fmriprep/cli/workflow.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/workflow.py#L237

Added line #L237 was not covered by tests
2 changes: 1 addition & 1 deletion fmriprep/utils/bids.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,21 @@
patterns = _patterns

derivs_cache = defaultdict(list, {})
layout = BIDSLayout(derivatives_dir, config=['bids', 'derivatives'], validate=False)

Check warning on line 59 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L59

Added line #L59 was not covered by tests
derivatives_dir = Path(derivatives_dir)

# search for both boldrefs
for k, q in spec['baseline'].items():

Check warning on line 63 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L63

Added line #L63 was not covered by tests
query = {**q, **entities}
item = layout.get(return_type='filename', **query)
if not item:
continue
derivs_cache['%s_boldref' % k] = item[0] if len(item) == 1 else item

Check warning on line 68 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L68

Added line #L68 was not covered by tests

for xfm, q in spec['transforms'].items():
query = {**q, **entities}
if xfm == 'boldref2fmap':
query['to'] = fieldmap_id

Check warning on line 73 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L72-L73

Added lines #L72 - L73 were not covered by tests
item = layout.get(return_type='filename', **q)
if not item:
continue
Expand All @@ -91,9 +91,9 @@
'*_mixing.tsv',
'*_timeseries.tsv',
)
ignore_file = Path(deriv_dir) / '.bidsignore'

Check warning on line 94 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L94

Added line #L94 was not covered by tests

ignore_file.write_text('\n'.join(bids_ignore) + '\n')

Check warning on line 96 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L96

Added line #L96 was not covered by tests


def write_derivative_description(bids_dir, deriv_dir):
Expand Down Expand Up @@ -228,14 +228,14 @@
ignored_subs = all_subs.difference(selected_subs)
if ignored_subs:
for sub in ignored_subs:
validator_config_dict['ignoredFiles'].append('/sub-%s/**' % sub)

Check warning on line 231 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L231

Added line #L231 was not covered by tests
with tempfile.NamedTemporaryFile(mode='w+', suffix='.json') as temp:
temp.write(json.dumps(validator_config_dict))
temp.flush()
try:
subprocess.check_call(['bids-validator', str(bids_dir), '-c', temp.name]) # noqa: S603, S607
subprocess.check_call(['bids-validator', str(bids_dir), '-c', temp.name]) # noqa: S607

Check warning on line 236 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L236

Added line #L236 was not covered by tests
except FileNotFoundError:
print('bids-validator does not appear to be installed', file=sys.stderr)

Check warning on line 238 in fmriprep/utils/bids.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/utils/bids.py#L238

Added line #L238 was not covered by tests


def check_pipeline_version(pipeline_name, cvers, data_desc):
Expand Down
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ extend-select = [
extend-ignore = [
"S311", # We are not using random for cryptographic purposes
"ISC001",
"S603",
]

[tool.ruff.lint.flake8-quotes]
Expand All @@ -185,6 +186,7 @@ inline-quotes = "single"
"*/test_*.py" = ["S101"]
"fmriprep/utils/debug.py" = ["A002", "T100"]
"docs/conf.py" = ["A001"]
"docs/sphinxext/github_link.py" = ["BLE001"]

[tool.ruff.format]
quote-style = "single"
45 changes: 41 additions & 4 deletions wrapper/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,47 @@ universal = true
# Developer tool configurations
#

# Disable black
[tool.black]
exclude = ".*"

[tool.ruff]
line-length = 99
target-version = ['py39']
skip-string-normalization = true

[tool.isort]
profile = 'black'
[tool.ruff.lint]
extend-select = [
"F",
"E",
"W",
"I",
"YTT",
"BLE",
"B",
"A",
# "CPY",
"C4",
"DTZ",
"T10",
# "EM",
"EXE",
"FA",
"ISC",
"ICN",
"PT",
"Q",
]
extend-ignore = [
"S311", # We are not using random for cryptographic purposes
"ISC001",
]

[tool.ruff.lint.flake8-quotes]
inline-quotes = "single"

[tool.ruff.lint.extend-per-file-ignores]
"*/test_*.py" = ["S101"]
"fmriprep/utils/debug.py" = ["A002", "T100"]
"docs/conf.py" = ["A001"]

[tool.ruff.format]
quote-style = "single"
34 changes: 17 additions & 17 deletions wrapper/src/fmriprep_docker/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def _run(args, stdout=None, stderr=None):

# De-fang Python 2's input - we don't eval user input
try:
input = raw_input
input = raw_input # noqa: A001
except NameError:
pass

Expand All @@ -116,7 +116,7 @@ def check_docker():
if e.errno == ENOENT:
return -1
raise e
if ret.stderr.startswith(b"Cannot connect to the Docker daemon."):
if ret.stderr.startswith(b'Cannot connect to the Docker daemon.'):
return 0
return 1

Expand Down Expand Up @@ -154,11 +154,11 @@ def _get_posargs(usage):
line = targ.lstrip()
if line.startswith('usage'):
continue
if line[0].isalnum() or line[0] == "{":
if line[0].isalnum() or line[0] == '{':
posargs.append(line)
elif line[0] == '[' and (line[1].isalnum() or line[1] == "{"):
elif line[0] == '[' and (line[1].isalnum() or line[1] == '{'):
posargs.append(line)
return " ".join(posargs)
return ' '.join(posargs)

# Matches all flags with up to two nested square brackets
# I'm sorry.
Expand Down Expand Up @@ -203,7 +203,7 @@ def _get_posargs(usage):
'w',
}

assert overlap == expected_overlap, "Clobbering options: {}".format(
assert overlap == expected_overlap, 'Clobbering options: {}'.format(
', '.join(overlap - expected_overlap)
)

Expand Down Expand Up @@ -263,15 +263,15 @@ class ToDict(argparse.Action):
def __call__(self, parser, namespace, values, option_string=None):
d = {}
for kv in values:
k, v = kv.split("=")
k, v = kv.split('=')
d[k] = os.path.abspath(v)
setattr(namespace, self.dest, d)

def _is_file(path, parser):
"""Ensure a given path exists and it is a file."""
path = os.path.abspath(path)
if not os.path.isfile(path):
raise parser.error("Path should point to a file (or symlink of file): <%s>." % path)
raise parser.error('Path should point to a file (or symlink of file): <%s>.' % path)
return path

parser = argparse.ArgumentParser(
Expand All @@ -288,7 +288,7 @@ def _is_file(path, parser):
)

parser.add_argument(
'-h', '--help', action='store_true', help="show this help message and exit"
'-h', '--help', action='store_true', help='show this help message and exit'
)
parser.add_argument(
'--version', action='store_true', help="show program's version number and exit"
Expand Down Expand Up @@ -319,7 +319,7 @@ def _is_file(path, parser):
)
g_wrap.add_argument(
'--output-spaces',
nargs="*",
nargs='*',
)

g_wrap.add_argument(
Expand Down Expand Up @@ -370,8 +370,8 @@ def _is_file(path, parser):
)
g_dev.add_argument(
'--patch',
nargs="+",
metavar="PACKAGE=PATH",
nargs='+',
metavar='PACKAGE=PATH',
action=ToDict,
help='Sequence of PACKAGE=PATH specifications to patch a Python package into the '
'container Python environment.',
Expand Down Expand Up @@ -431,7 +431,7 @@ def main():
if opts.help:
parser.print_help()
if check == -1:
print("fmriprep: Could not find docker command... Is it installed?")
print('fmriprep: Could not find docker command... Is it installed?')
else:
print("fmriprep: Make sure you have permission to run 'docker'")
return 1
Expand Down Expand Up @@ -477,7 +477,7 @@ def main():
return 0

ret = subprocess.run(
['docker', 'version', '--format', "{{.Server.Version}}"], stdout=subprocess.PIPE
['docker', 'version', '--format', '{{.Server.Version}}'], stdout=subprocess.PIPE
)
docker_version = ret.stdout.decode('ascii').strip()

Expand Down Expand Up @@ -566,7 +566,7 @@ def main():
if space.split(':')[0] not in (TF_TEMPLATES + NONSTANDARD_REFERENCES):
tpl = os.path.basename(space)
if not tpl.startswith('tpl-'):
raise RuntimeError("Custom template %s requires a `tpl-` prefix" % tpl)
raise RuntimeError('Custom template %s requires a `tpl-` prefix' % tpl)
target = '/home/fmriprep/.cache/templateflow/' + tpl
command.extend(['-v', ':'.join((os.path.abspath(space), target, 'ro'))])
spaces.append(tpl[4:])
Expand Down Expand Up @@ -599,10 +599,10 @@ def main():
command.extend(main_args)
command.extend(unknown_args)

print("RUNNING: " + ' '.join(command))
print('RUNNING: ' + ' '.join(command))
ret = subprocess.run(command)
if ret.returncode:
print("fMRIPrep: Please report errors to {}".format(__bugreports__))
print('fMRIPrep: Please report errors to {}'.format(__bugreports__))
return ret.returncode


Expand Down
Loading