From 2d6a11d3e814e73b7ae5da71918ec4fb0265c77f Mon Sep 17 00:00:00 2001 From: Taylor Salo Date: Wed, 28 Aug 2024 13:07:28 -0400 Subject: [PATCH] Deploy to DockerHub with GitHub Action (#46) * Update parser.py * Update config.yml * Run isort. * Update. * Update pyproject.toml * update * fix. * Update get_data.py * Deploy to DockerHub with GitHub Action. * Update parser.py * Apply suggestions from code review Co-authored-by: Chris Markiewicz * Update action versions. --------- Co-authored-by: Chris Markiewicz --- .circleci/config.yml | 73 +++++++++------------ .circleci/get_data.py | 1 + .github/workflows/docker.yml | 46 +++++++++++++ pyproject.toml | 14 +++- src/fmripost_aroma/cli/parser.py | 64 +++++++++--------- src/fmripost_aroma/cli/run.py | 13 +--- src/fmripost_aroma/reports/core.py | 3 +- src/fmripost_aroma/tests/test_base.py | 3 +- src/fmripost_aroma/tests/test_cli.py | 13 ++-- src/fmripost_aroma/tests/test_utils_bids.py | 1 + src/fmripost_aroma/tests/tests.py | 3 +- src/fmripost_aroma/workflows/base.py | 10 +-- 12 files changed, 142 insertions(+), 102 deletions(-) create mode 100644 .github/workflows/docker.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 60adb12..963a305 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -5,7 +5,7 @@ orbs: .dockersetup: &dockersetup docker: - image: cimg/python:3.12 - working_directory: /src/fmripost_aroma + working_directory: /src/fmripost-aroma runinstall: &runinstall name: Install fMRIPost-AROMA @@ -15,8 +15,8 @@ runinstall: &runinstall VERSION="$CIRCLE_TAG" fi git checkout $CIRCLE_BRANCH - echo "${VERSION}" > /src/fmripost_aroma/src/fmripost_aroma/VERSION - echo "include src/fmripost_aroma/VERSION" >> /src/fmripost_aroma/src/fmripost_aroma/MANIFEST.in + echo "${VERSION}" > /src/fmripost-aroma/src/fmripost_aroma/VERSION + echo "include src/fmripost_aroma/VERSION" >> /src/fmripost-aroma/src/fmripost_aroma/MANIFEST.in pip install .[tests] --progress-bar off # Precaching fonts, set 'Agg' as default backend for matplotlib @@ -48,12 +48,12 @@ jobs: - run: name: Download ds005115_raw test data command: | - cd /src/fmripost_aroma/.circleci + cd /src/fmripost-aroma/.circleci python get_data.py $PWD/data ds005115_raw - save_cache: key: ds005115_raw-01 paths: - - /src/fmripost_aroma/.circleci/data/ds005115_raw + - /src/fmripost-aroma/.circleci/data/ds005115_raw download_ds005115_resampling: <<: *dockersetup @@ -65,12 +65,12 @@ jobs: - run: name: Download ds005115_resampling test data command: | - cd /src/fmripost_aroma/.circleci + cd /src/fmripost-aroma/.circleci python get_data.py $PWD/data ds005115_resampling - save_cache: key: ds005115_resampling-01 paths: - - /src/fmripost_aroma/.circleci/data/ds005115_resampling + - /src/fmripost-aroma/.circleci/data/ds005115_resampling download_ds005115_deriv_no_mni6: <<: *dockersetup @@ -82,12 +82,12 @@ jobs: - run: name: Download ds005115_deriv_no_mni6 test data command: | - cd /src/fmripost_aroma/.circleci + cd /src/fmripost-aroma/.circleci python get_data.py $PWD/data ds005115_deriv_no_mni6 - save_cache: key: ds005115_deriv_no_mni6-01 paths: - - /src/fmripost_aroma/.circleci/data/ds005115_deriv_no_mni6 + - /src/fmripost-aroma/.circleci/data/ds005115_deriv_no_mni6 download_ds005115_deriv_mni6: <<: *dockersetup @@ -99,12 +99,12 @@ jobs: - run: name: Download ds005115_deriv_mni6 test data command: | - cd /src/fmripost_aroma/.circleci + cd /src/fmripost-aroma/.circleci python get_data.py $PWD/data ds005115_deriv_mni6 - save_cache: key: ds005115_deriv_mni6-01 paths: - - /src/fmripost_aroma/.circleci/data/ds005115_deriv_mni6 + - /src/fmripost-aroma/.circleci/data/ds005115_deriv_mni6 ds005115_deriv_only: <<: *dockersetup @@ -117,18 +117,18 @@ jobs: name: Test the PYAFQ standalone recon workflow no_output_timeout: 1h command: | - pytest -rP -o log_cli=true -m "pyafq_recon_full" --cov-config=/src/fmripost_aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost_aroma/.circleci/data --output_dir=/src/fmripost_aroma/.circleci/out --working_dir=/src/fmripost_aroma/.circleci/work fmripost_aroma + pytest -rP -o log_cli=true -m "pyafq_recon_full" --cov-config=/src/fmripost-aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost-aroma/.circleci/data --output_dir=/src/fmripost-aroma/.circleci/out --working_dir=/src/fmripost-aroma/.circleci/work fmripost_aroma mkdir /src/coverage - mv /src/fmripost_aroma/.coverage /src/coverage/.coverage.pyafq_recon_full + mv /src/fmripost-aroma/.coverage /src/coverage/.coverage.pyafq_recon_full # remove nifti files before uploading artifacts - find /src/fmripost_aroma/.circleci/out/ -name "*.nii.gz" -type f -delete - find /src/fmripost_aroma/.circleci/out/ -name "*.fib.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.nii.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.fib.gz" -type f -delete - persist_to_workspace: root: /src/coverage/ paths: - .coverage.pyafq_recon_full - store_artifacts: - path: /src/fmripost_aroma/.circleci/out/pyafq_recon_full/ + path: /src/fmripost-aroma/.circleci/out/pyafq_recon_full/ ds005115_deriv_and_raw: <<: *dockersetup @@ -141,18 +141,18 @@ jobs: name: Test the PYAFQ workflow with mrtrix tractography no_output_timeout: 1h command: | - pytest -rP -o log_cli=true -m "pyafq_recon_external_trk" --cov-config=/src/fmripost_aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost_aroma/.circleci/data --output_dir=/src/fmripost_aroma/.circleci/out --working_dir=/src/fmripost_aroma/.circleci/work fmripost_aroma + pytest -rP -o log_cli=true -m "pyafq_recon_external_trk" --cov-config=/src/fmripost-aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost-aroma/.circleci/data --output_dir=/src/fmripost-aroma/.circleci/out --working_dir=/src/fmripost-aroma/.circleci/work fmripost_aroma mkdir /src/coverage - mv /src/fmripost_aroma/.coverage /src/coverage/.coverage.pyafq_recon_external_trk + mv /src/fmripost-aroma/.coverage /src/coverage/.coverage.pyafq_recon_external_trk # remove nifti files before uploading artifacts - find /src/fmripost_aroma/.circleci/out/ -name "*.nii.gz" -type f -delete - find /src/fmripost_aroma/.circleci/out/ -name "*.fib.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.nii.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.fib.gz" -type f -delete - persist_to_workspace: root: /src/coverage/ paths: - .coverage.pyafq_recon_external_trk - store_artifacts: - path: /src/fmripost_aroma/.circleci/out/pyafq_recon_external_trk/ + path: /src/fmripost-aroma/.circleci/out/pyafq_recon_external_trk/ ds005115_resampling_and_raw: <<: *dockersetup @@ -165,18 +165,18 @@ jobs: name: Test scalar_mapping workflow no_output_timeout: 1h command: | - pytest -rP -o log_cli=true -m "scalar_mapper" --cov-config=/src/fmripost_aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost_aroma/.circleci/data --output_dir=/src/fmripost_aroma/.circleci/out --working_dir=/src/fmripost_aroma/.circleci/work fmripost_aroma + pytest -rP -o log_cli=true -m "scalar_mapper" --cov-config=/src/fmripost-aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost-aroma/.circleci/data --output_dir=/src/fmripost-aroma/.circleci/out --working_dir=/src/fmripost-aroma/.circleci/work fmripost_aroma mkdir /src/coverage - mv /src/fmripost_aroma/.coverage /src/coverage/.coverage.scalar_mapper + mv /src/fmripost-aroma/.coverage /src/coverage/.coverage.scalar_mapper # remove nifti files before uploading artifacts - find /src/fmripost_aroma/.circleci/out/ -name "*.nii.gz" -type f -delete - find /src/fmripost_aroma/.circleci/out/ -name "*.fib.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.nii.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.fib.gz" -type f -delete - persist_to_workspace: root: /src/coverage/ paths: - .coverage.scalar_mapper - store_artifacts: - path: /src/fmripost_aroma/.circleci/out/scalar_mapper/ + path: /src/fmripost-aroma/.circleci/out/scalar_mapper/ pytests: <<: *dockersetup @@ -191,18 +191,18 @@ jobs: - run: name: Test the DIPY recon workflows command: | - pytest -rP -o log_cli=true -m "amico_noddi" --cov-config=/src/fmripost_aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost_aroma/.circleci/data --output_dir=/src/fmripost_aroma/.circleci/out --working_dir=/src/fmripost_aroma/.circleci/work fmripost_aroma + pytest -rP -o log_cli=true -m "amico_noddi" --cov-config=/src/fmripost-aroma/pyproject.toml --cov-append --cov-report term-missing --cov=fmripost_aroma --data_dir=/src/fmripost-aroma/.circleci/data --output_dir=/src/fmripost-aroma/.circleci/out --working_dir=/src/fmripost-aroma/.circleci/work fmripost_aroma mkdir /src/coverage - mv /src/fmripost_aroma/.coverage /src/coverage/.coverage.amico_noddi + mv /src/fmripost-aroma/.coverage /src/coverage/.coverage.amico_noddi # remove nifti files before uploading artifacts - find /src/fmripost_aroma/.circleci/out/ -name "*.nii.gz" -type f -delete - find /src/fmripost_aroma/.circleci/out/ -name "*.fib.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.nii.gz" -type f -delete + find /src/fmripost-aroma/.circleci/out/ -name "*.fib.gz" -type f -delete - persist_to_workspace: root: /src/coverage/ paths: - .coverage.amico_noddi - store_artifacts: - path: /src/fmripost_aroma/.circleci/out/amico_noddi/ + path: /src/fmripost-aroma/.circleci/out/amico_noddi/ merge_coverage: <<: *dockersetup @@ -234,7 +234,7 @@ jobs: TZ: "/usr/share/zoneinfo/America/New_York" docker: - image: cimg/base:2020.09 - working_directory: /tmp/src/fmripost_aroma + working_directory: /tmp/src/fmripost-aroma steps: - checkout - setup_remote_docker: @@ -379,12 +379,3 @@ workflows: only: main tags: only: /.*/ - - - build_and_deploy: - requires: - - deployable - filters: - branches: - only: main - tags: - only: /.*/ diff --git a/.circleci/get_data.py b/.circleci/get_data.py index 390309d..0a5f22b 100755 --- a/.circleci/get_data.py +++ b/.circleci/get_data.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 """Download test data.""" + import sys from fmripost_aroma.tests.utils import download_test_data diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml new file mode 100644 index 0000000..2e93152 --- /dev/null +++ b/.github/workflows/docker.yml @@ -0,0 +1,46 @@ +name: Publish Docker image + +on: + push: + branches: [main] + release: + types: [published] + +jobs: + push_to_registry: + name: Push Docker image to Docker Hub + runs-on: ubuntu-latest + permissions: + packages: write + contents: read + attestations: write + id-token: write + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3.6.1 + + - name: Log in to Docker Hub + uses: docker/login-action@v3.3.0 + with: + username: niprepsbot + password: ${{ secrets.DOCKER_TOKEN }} + + - name: Extract metadata (tags, labels) for Docker + id: meta + uses: docker/metadata-action@v5.5.1 + with: + images: nipreps/fmripost-aroma + + - name: Build and push Docker image + uses: docker/build-push-action@v6.7.0 + with: + context: . + push: true + tags: | + nipreps/fmripost-aroma:unstable + ${{ github.event_name == 'release' && 'nipreps/fmripost-aroma:latest' || '' }} + ${{ github.event_name == 'release' && 'nipreps/fmripost-aroma:${{ github.event.release.tag_name }}' || '' }} diff --git a/pyproject.toml b/pyproject.toml index 8bec10f..7bcd7bc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,13 +108,13 @@ dependencies = [ [tool.hatch.envs.style.scripts] fix = [ "black src/", - "isort src/", "ruff check --fix src/", + "isort src/", ] check = [ "black --check --diff src/", - "isort --check --diff src/", "ruff check --diff src/", + "isort --check --diff src/", ] [[tool.hatch.envs.test.matrix]] @@ -169,6 +169,10 @@ ignore = [ "S311", # We are not using random for cryptographic purposes "ISC001", "S603", + "PT023", + "S113", + "S202", + "S602", ] [tool.ruff.lint.flake8-quotes] @@ -183,6 +187,12 @@ inline-quotes = "single" [tool.ruff.format] quote-style = "single" +[tool.isort] +profile = "black" +multi_line_output = 3 +src_paths = ["isort", "test"] +known_local_folder = ["fmripost_aroma"] + [tool.pytest.ini_options] addopts = '-m "not integration"' markers = [ diff --git a/src/fmripost_aroma/cli/parser.py b/src/fmripost_aroma/cli/parser.py index b04130c..2ce6319 100644 --- a/src/fmripost_aroma/cli/parser.py +++ b/src/fmripost_aroma/cli/parser.py @@ -152,6 +152,38 @@ def _bids_filter(value, parser): ), ) + g_aroma = parser.add_argument_group('Options for running ICA-AROMA') + g_aroma.add_argument( + '--melodic-dimensionality', + dest='melodic_dim', + action='store', + default=0, + type=int, + help=( + 'Exact or maximum number of MELODIC components to estimate ' + '(positive = exact, negative = maximum)' + ), + ) + g_aroma.add_argument( + '--error-on-warnings', + dest='err_on_warn', + action='store_true', + default=False, + help=( + 'Raise an error if ICA-AROMA does not produce sensible output ' + '(e.g., if all the components are classified as signal or noise)' + ), + ) + g_aroma.add_argument( + '--denoising-method', + action='store', + nargs='+', + choices=['aggr', 'nonaggr', 'orthaggr'], + default=None, + dest='denoise_method', + help='Denoising method to apply, if any.', + ) + g_bids = parser.add_argument_group('Options for filtering BIDS queries') g_bids.add_argument( '--skip_bids_validation', @@ -345,38 +377,6 @@ def _bids_filter(value, parser): ), ) - g_aroma = parser.add_argument_group('Options for running ICA_AROMA') - g_aroma.add_argument( - '--melodic-dimensionality', - dest='melodic_dim', - action='store', - default=0, - type=int, - help=( - 'Exact or maximum number of MELODIC components to estimate ' - '(positive = exact, negative = maximum)' - ), - ) - g_aroma.add_argument( - '--error-on-warnings', - dest='err_on_warn', - action='store_true', - default=False, - help=( - 'Raise an error if ICA_AROMA does not produce sensible output ' - '(e.g., if all the components are classified as signal or noise)' - ), - ) - g_aroma.add_argument( - '--denoising-method', - action='store', - nargs='+', - choices=['aggr', 'nonaggr', 'orthaggr'], - default=None, - dest='denoise_method', - help='Denoising method to apply, if any.', - ) - g_carbon = parser.add_argument_group('Options for carbon usage tracking') g_carbon.add_argument( '--track-carbon', diff --git a/src/fmripost_aroma/cli/run.py b/src/fmripost_aroma/cli/run.py index cab8652..b5bbfa5 100644 --- a/src/fmripost_aroma/cli/run.py +++ b/src/fmripost_aroma/cli/run.py @@ -152,10 +152,7 @@ def main(): from fmripost_aroma.utils.telemetry import process_crashfile crashfolders = [ - config.execution.output_dir - / f'sub-{s}' - / 'log' - / config.execution.run_uuid + config.execution.output_dir / f'sub-{s}' / 'log' / config.execution.run_uuid for s in config.execution.participant_label ] for crashfolder in crashfolders: @@ -199,9 +196,7 @@ def main(): dseg_tsv = str(api.get('fsaverage', suffix='dseg', extension=['.tsv'])) _copy_any(dseg_tsv, str(config.execution.output_dir / 'desc-aseg_dseg.tsv')) - _copy_any( - dseg_tsv, str(config.execution.output_dir / 'desc-aparcaseg_dseg.tsv') - ) + _copy_any(dseg_tsv, str(config.execution.output_dir / 'desc-aparcaseg_dseg.tsv')) errno = 0 finally: # Code Carbon @@ -219,9 +214,7 @@ def main(): output_dir=config.execution.output_dir, run_uuid=config.execution.run_uuid, ) - write_derivative_description( - config.execution.bids_dir, config.execution.output_dir - ) + write_derivative_description(config.execution.bids_dir, config.execution.output_dir) write_bidsignore(config.execution.output_dir) if sentry_sdk is not None and failed_reports: diff --git a/src/fmripost_aroma/reports/core.py b/src/fmripost_aroma/reports/core.py index 4bf2ab2..daf2cf8 100644 --- a/src/fmripost_aroma/reports/core.py +++ b/src/fmripost_aroma/reports/core.py @@ -22,9 +22,10 @@ # from pathlib import Path -from fmripost_aroma import config, data from nireports.assembler.report import Report +from fmripost_aroma import config, data + def run_reports( output_dir, diff --git a/src/fmripost_aroma/tests/test_base.py b/src/fmripost_aroma/tests/test_base.py index 2868cbd..0534018 100644 --- a/src/fmripost_aroma/tests/test_base.py +++ b/src/fmripost_aroma/tests/test_base.py @@ -1,8 +1,9 @@ """Tests for fmripost_aroma.workflows.""" -from fmripost_aroma import config from fmriprep.workflows.tests import mock_config +from fmripost_aroma import config + def test_init_ica_aroma_wf(tmp_path_factory): from fmripost_aroma.workflows.aroma import init_ica_aroma_wf diff --git a/src/fmripost_aroma/tests/test_cli.py b/src/fmripost_aroma/tests/test_cli.py index 8b812fc..81e8545 100644 --- a/src/fmripost_aroma/tests/test_cli.py +++ b/src/fmripost_aroma/tests/test_cli.py @@ -5,6 +5,7 @@ from unittest.mock import patch import pytest + from fmripost_aroma.cli import run from fmripost_aroma.cli.parser import parse_args from fmripost_aroma.cli.workflow import build_boilerplate, build_workflow @@ -17,8 +18,8 @@ from fmripost_aroma.utils.bids import write_derivative_description -@pytest.mark.integration() -@pytest.mark.ds005115_deriv_only() +@pytest.mark.integration +@pytest.mark.ds005115_deriv_only def test_ds005115_deriv_only(data_dir, output_dir, working_dir): """Run fMRIPost-AROMA on ds005115 fMRIPrep derivatives with MNI152NLin6Asym-space data.""" test_name = 'test_ds005115_deriv_only' @@ -43,8 +44,8 @@ def test_ds005115_deriv_only(data_dir, output_dir, working_dir): ) -@pytest.mark.integration() -@pytest.mark.ds005115_deriv_and_raw() +@pytest.mark.integration +@pytest.mark.ds005115_deriv_and_raw def test_ds005115_deriv_and_raw(data_dir, output_dir, working_dir): """Run fMRIPost-AROMA on ds005115 raw BIDS + fMRIPrep derivatives w/o MNI152NLin6Asym data.""" test_name = 'test_ds005115_deriv_and_raw' @@ -72,8 +73,8 @@ def test_ds005115_deriv_and_raw(data_dir, output_dir, working_dir): ) -@pytest.mark.integration() -@pytest.mark.ds005115_resampling_and_raw() +@pytest.mark.integration +@pytest.mark.ds005115_resampling_and_raw def test_ds005115_resampling_and_raw(data_dir, output_dir, working_dir): """Run fMRIPost-AROMA on ds005115 raw BIDS + resampling-level fMRIPrep derivatives.""" test_name = 'test_ds005115_resampling_and_raw' diff --git a/src/fmripost_aroma/tests/test_utils_bids.py b/src/fmripost_aroma/tests/test_utils_bids.py index 7446411..7c62ba7 100644 --- a/src/fmripost_aroma/tests/test_utils_bids.py +++ b/src/fmripost_aroma/tests/test_utils_bids.py @@ -4,6 +4,7 @@ import pytest from bids.layout import BIDSLayout, BIDSLayoutIndexer + from fmripost_aroma.tests.utils import get_test_data_path from fmripost_aroma.utils import bids as xbids diff --git a/src/fmripost_aroma/tests/tests.py b/src/fmripost_aroma/tests/tests.py index 01596aa..3add0f5 100644 --- a/src/fmripost_aroma/tests/tests.py +++ b/src/fmripost_aroma/tests/tests.py @@ -28,9 +28,10 @@ from pathlib import Path from tempfile import mkdtemp -from fmripost_aroma.data import load as load_data from toml import loads +from fmripost_aroma.data import load as load_data + @contextmanager def mock_config(): diff --git a/src/fmripost_aroma/workflows/base.py b/src/fmripost_aroma/workflows/base.py index ee2f202..2ef7825 100644 --- a/src/fmripost_aroma/workflows/base.py +++ b/src/fmripost_aroma/workflows/base.py @@ -72,10 +72,7 @@ def init_fmripost_aroma_wf(): single_subject_wf = init_single_subject_wf(subject_id) single_subject_wf.config['execution']['crashdump_dir'] = str( - config.execution.output_dir - / f'sub-{subject_id}' - / 'log' - / config.execution.run_uuid + config.execution.output_dir / f'sub-{subject_id}' / 'log' / config.execution.run_uuid ) for node in single_subject_wf._get_all_nodes(): node.config = deepcopy(single_subject_wf.config) @@ -84,10 +81,7 @@ def init_fmripost_aroma_wf(): # Dump a copy of the config file into the log directory log_dir = ( - config.execution.output_dir - / f'sub-{subject_id}' - / 'log' - / config.execution.run_uuid + config.execution.output_dir / f'sub-{subject_id}' / 'log' / config.execution.run_uuid ) log_dir.mkdir(exist_ok=True, parents=True) config.to_filename(log_dir / 'fmripost_aroma.toml')