From a0df711013ff51ca494773b2aa422d234d7aa50b Mon Sep 17 00:00:00 2001 From: Remi Gau Date: Sun, 22 Jan 2023 15:18:15 +0100 Subject: [PATCH] [ENH] update config (#126) * do not unlock * retry * remove debug mode from demos * add source file to json * add circle ci context * update test * fix relative path * fix test * update config * fix tests * revert to desc * reset database where appropriate * fix repo name * do system test on dataset only on circle ci --- .circleci/config.yml | 48 ++++++++++++++++++++--- .github/workflows/system_tests.yml | 2 +- Dockerfile | 9 +---- Makefile | 13 ++---- bidsmreye/bids_utils.py | 32 ++++++++++++--- bidsmreye/bidsmreye.py | 3 ++ bidsmreye/config/config_pybids.json | 4 ++ bidsmreye/config/default_filter_file.json | 1 + bidsmreye/configuration.py | 7 +--- bidsmreye/prepare_data.py | 32 +++++++-------- tests/test_bids_utils.py | 44 +++++++++++++++++++++ tests/test_utils.py | 7 +++- 12 files changed, 148 insertions(+), 54 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4edc0fe..2a835f1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -11,8 +11,18 @@ jobs: - run: name: Build Docker image command: | - wget https://raw.githubusercontent.com/bids-apps/maintenance-tools/main/circleci/build_docker.sh - bash build_docker.sh + set -eux -o pipefail + # make sure we have a lowercase repo + user_name=cpplab + repo_name=$(echo "${CIRCLE_PROJECT_REPONAME}" | tr '[:upper:]' '[:lower:]') + if [[ -e "${HOME}/docker/image.tar" ]]; then + docker load -i "${HOME}/docker/image.tar" + fi + git describe --tags --always > version + docker build -t "${user_name}/${repo_name}" . + mkdir -p "${HOME}/docker" + docker save "${user_name}/${repo_name}" > "${HOME}/docker/image.tar" + docker images - persist_to_workspace: root: /home/circleci @@ -68,21 +78,26 @@ jobs: - run: name: print version command: | + user_name=cpplab + repo_name=$(echo "${CIRCLE_PROJECT_REPONAME}" | tr '[:upper:]' '[:lower:]') docker run -ti --rm \ -v /tmp/workspace/data/ds002799/derivatives/fmriprep:/bids_dataset \ - cpp-lln-lab/bidsmreye --version + ${user_name}/${repo_name} --version - run: name: run all command: | + user_name=cpplab + repo_name=$(echo "${CIRCLE_PROJECT_REPONAME}" | tr '[:upper:]' '[:lower:]') docker run -ti --rm \ -v /tmp/workspace/data/ds002799/derivatives/fmriprep:/bids_dataset \ -v ${HOME}/outputs:/outputs \ - cpp-lln-lab/bidsmreye \ + ${user_name}/${repo_name} \ /bids_dataset \ /outputs \ participant \ --participant_label 302 307 \ --space MNI152NLin2009cAsym \ + --reset_database \ --action all no_output_timeout: 6h - store_artifacts: @@ -99,8 +114,27 @@ jobs: - run: name: push to dockerhub command: | - wget https://raw.githubusercontent.com/bids-apps/maintenance-tools/main/circleci/push_docker.sh - bash push_docker.sh + set -ex -o pipefail + if [[ -n "${DOCKER_TOKEN}" ]]; then + # make sure we have a lowercase repo + user_name=cpplab + repo_name=$(echo "${CIRCLE_PROJECT_REPONAME}" | tr '[:upper:]' '[:lower:]') + if [[ -n "${DOCKER_TOKEN}" ]]; then + echo "${DOCKER_TOKEN}" | docker login -u "${DOCKER_USER}" --password-stdin + : "Pushing to DockerHub ${user_name}/${repo_name}:unstable" + docker tag "${user_name}/${repo_name}" "${user_name}/${repo_name}:unstable" + docker push "${user_name}/${repo_name}:unstable" + if [[ -n "${CIRCLE_TAG}" ]]; then + : "Pushing to DockerHub ${user_name}/${repo_name}:${CIRCLE_TAG}" + docker push "${user_name}/${repo_name}:latest" + docker tag "${user_name}/${repo_name}" "${user_name}/${repo_name}:${CIRCLE_TAG}" + docker push "${user_name}/${repo_name}:${CIRCLE_TAG}" + fi + fi + else + : "No DOCKER_TOKEN, skipping push to DockerHub" + exit 1 + fi workflows: build-test-deploy: @@ -114,6 +148,8 @@ workflows: - get_data - deploy: + context: + - DOCKER_HUB requires: - test filters: diff --git a/.github/workflows/system_tests.yml b/.github/workflows/system_tests.yml index 6e7a377..0480128 100644 --- a/.github/workflows/system_tests.yml +++ b/.github/workflows/system_tests.yml @@ -19,7 +19,7 @@ jobs: strategy: matrix: python-version: ['3.9'] - dataset: [demo, ds002799] + dataset: [demo] steps: diff --git a/Dockerfile b/Dockerfile index b709cc7..2982add 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,14 +14,7 @@ COPY [".", "/home/neuro/bidsMReye"] RUN pip install --upgrade pip && \ pip3 install -e . -RUN bidsmreye_model && \ - bidsmreye_model --model_name 1_guided_fixations && \ - bidsmreye_model --model_name 2_pursuit && \ - bidsmreye_model --model_name 3_openclosed && \ - bidsmreye_model --model_name 3_pursuit && \ - bidsmreye_model --model_name 4_pursuit && \ - bidsmreye_model --model_name 5_free_viewing && \ - bidsmreye_model --model_name 1to5 +RUN bidsmreye_model ENTRYPOINT [ "//home/neuro/bidsMReye/entrypoint.sh" ] COPY ["./docker/entrypoint.sh", \ diff --git a/Makefile b/Makefile index 1857d07..b781d61 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,6 @@ prepare: tests/data/moae_fmriprep ## demo: prepares the data of MOAE dataset participant \ --action prepare \ -vv \ - --debug \ --reset_database \ --non_linear_coreg @@ -151,8 +150,6 @@ generalize: ## demo: predicts labels of MOAE dataset participant \ --action generalize \ -vv \ - --debug \ - --reset_database \ --non_linear_coreg @@ -174,9 +171,9 @@ ds002799_prepare: get_ds002799 $$PWD/outputs/ds002799/derivatives \ participant \ --action prepare \ - --debug \ --participant_label 302 307 \ --space MNI152NLin2009cAsym \ + --reset_database \ --run 1 2 @@ -185,7 +182,6 @@ ds002799_generalize: $$PWD/outputs/ds002799/derivatives \ participant \ --action generalize \ - --debug \ --participant_label 302 307 \ --space MNI152NLin2009cAsym \ --run 1 2 @@ -196,10 +192,10 @@ ds002799: clean-ds002799 get_ds002799 $$PWD/outputs/ds002799/derivatives \ participant \ --action all \ - --debug \ --participant_label 302 307 \ --space MNI152NLin2009cAsym \ --run 1 2 \ + --reset_database \ -vv ## DOCKER @@ -224,7 +220,6 @@ docker_prepare_data: /home/neuro/outputs/ \ participant \ --action prepare \ - --debug \ --reset_database docker_generalize: @@ -238,7 +233,7 @@ docker_generalize: --action generalize docker_ds002799: get_ds002799 - datalad unlock $$PWD/tests/data/ds002799/derivatives/fmriprep/sub-30[27]/ses-*/func/*run-*preproc*bold* +# datalad unlock $$PWD/tests/data/ds002799/derivatives/fmriprep/sub-30[27]/ses-*/func/*run-*preproc*bold* docker run --rm -it \ -v $$PWD/tests/data/ds002799/derivatives/fmriprep:/home/neuro/data \ -v $$PWD/outputs/ds002799/derivatives:/home/neuro/outputs/ \ @@ -247,8 +242,8 @@ docker_ds002799: get_ds002799 /home/neuro/outputs/ \ participant \ --action all \ - --debug \ --participant_label 302 307 \ --space MNI152NLin2009cAsym \ --run 1 2 \ + --reset_database \ -vv diff --git a/bidsmreye/bids_utils.py b/bidsmreye/bids_utils.py index 7be094a..4c7d80e 100644 --- a/bidsmreye/bids_utils.py +++ b/bidsmreye/bids_utils.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import Any +import nibabel as nib from bids import BIDSLayout # type: ignore from . import _version @@ -112,7 +113,10 @@ def create_bidsname( def create_sidecar( - layout: BIDSLayout, filename: str, SamplingFrequency: float | None = None + layout: BIDSLayout, + filename: str, + SamplingFrequency: float | None = None, + source: str | None = None, ) -> None: """Create sidecar for the eye motion timeseries.""" if SamplingFrequency is None: @@ -120,14 +124,31 @@ def create_sidecar( content = { "SamplingFrequency": SamplingFrequency, } + if source is not None: + content["Sources"] = [source] # type: ignore sidecar_name = create_bidsname(layout, filename, "confounds_json") json.dump(content, open(sidecar_name, "w"), indent=4) + log.debug(f"sidecar saved to {sidecar_name}") + + +def save_sampling_frequency_to_json( + layout_out: BIDSLayout, img: str, source: str +) -> None: + func_img = nib.load(img) + header = func_img.header + sampling_frequency = header.get_zooms()[3] + if sampling_frequency <= 1: + log.warning(f"Found a repetition time of {sampling_frequency} seconds.") + create_sidecar( + layout_out, img, SamplingFrequency=1 / float(sampling_frequency), source=source + ) def get_dataset_layout( dataset_path: str | Path, - config: dict[str, str] | None = None, + config: str | list[str] | dict[str, str] | None = None, use_database: bool = False, + reset_database: bool = False, ) -> BIDSLayout: """Return a BIDSLayout object for the dataset at the given path. @@ -149,6 +170,7 @@ def get_dataset_layout( dataset_path = dataset_path.resolve() + pybids_config = config if config is None: pybids_config = get_pybids_config() @@ -156,10 +178,7 @@ def get_dataset_layout( if not use_database: return BIDSLayout( - dataset_path, - validate=False, - derivatives=False, - config=pybids_config, + dataset_path, validate=False, derivatives=False, config=pybids_config ) database_path = dataset_path.joinpath("pybids_db") @@ -169,6 +188,7 @@ def get_dataset_layout( derivatives=False, config=pybids_config, database_path=database_path, + reset_database=reset_database, ) diff --git a/bidsmreye/bidsmreye.py b/bidsmreye/bidsmreye.py index 8202bd4..d49fdd1 100755 --- a/bidsmreye/bidsmreye.py +++ b/bidsmreye/bidsmreye.py @@ -91,6 +91,9 @@ def bidsmreye( with open(Path(bids_filter_file)) as f: bids_filter = json.load(f) + if reset_database is None: + reset_database = False + cfg = Config( bids_dir, output_dir, diff --git a/bidsmreye/config/config_pybids.json b/bidsmreye/config/config_pybids.json index dea976e..70d3a0c 100644 --- a/bidsmreye/config/config_pybids.json +++ b/bidsmreye/config/config_pybids.json @@ -94,6 +94,10 @@ "name": "chunk", "pattern": "[_/\\\\]+chunk-([0-9]+)" }, + { + "name": "desc", + "pattern": "desc-([a-zA-Z0-9]+)" + }, { "name": "suffix", "pattern": "[._]*([a-zA-Z0-9]*?)\\.[^/\\\\]+$" diff --git a/bidsmreye/config/default_filter_file.json b/bidsmreye/config/default_filter_file.json index 2446afb..d0b5fb6 100644 --- a/bidsmreye/config/default_filter_file.json +++ b/bidsmreye/config/default_filter_file.json @@ -1,6 +1,7 @@ { "bold": { "datatype": "func", + "desc": "preproc", "suffix": "^bold$", "extension": "nii.*" }, diff --git a/bidsmreye/configuration.py b/bidsmreye/configuration.py index 01543ee..2ffcd0e 100644 --- a/bidsmreye/configuration.py +++ b/bidsmreye/configuration.py @@ -56,7 +56,7 @@ def _check_input_dir(self, attribute: str, value: Path) -> None: bids_filter: Any = field(kw_only=True, default=None) debug: str | bool | None = field(kw_only=True, default=None) - reset_database: str | bool | None = field(kw_only=True, default=None) + reset_database: bool = field(kw_only=True, default=False) non_linear_coreg: bool = field(kw_only=True, default=False) has_GPU: bool = False @@ -70,11 +70,6 @@ def __attrs_post_init__(self) -> None: if not isinstance(self.debug, (bool)): self.debug = converters.to_bool(self.debug) - if not self.reset_database: - self.reset_database = False - if not isinstance(self.reset_database, (bool)): - self.reset_database = converters.to_bool(self.reset_database) - if not self.run: self.run = [] diff --git a/bidsmreye/prepare_data.py b/bidsmreye/prepare_data.py index b5cdfc9..6934eb2 100644 --- a/bidsmreye/prepare_data.py +++ b/bidsmreye/prepare_data.py @@ -5,17 +5,16 @@ from pathlib import Path from typing import Any -import nibabel as nib import numpy as np from bids import BIDSLayout # type: ignore from deepmreye import preprocess from bidsmreye.bids_utils import check_layout from bidsmreye.bids_utils import create_bidsname -from bidsmreye.bids_utils import create_sidecar from bidsmreye.bids_utils import get_dataset_layout from bidsmreye.bids_utils import init_dataset from bidsmreye.bids_utils import list_subjects +from bidsmreye.bids_utils import save_sampling_frequency_to_json from bidsmreye.configuration import Config from bidsmreye.logging import bidsmreye_log from bidsmreye.utils import check_if_file_found @@ -142,28 +141,22 @@ def process_subject( coregister_and_extract_data(img, non_linear_coreg=cfg.non_linear_coreg) - report_name = create_bidsname(layout_out, img, "report") - deepmreye_mask_report = get_deepmreye_filename(layout_in, img, "report") + report_name = create_bidsname(layout_out, filename=img, filetype="report") + deepmreye_mask_report = get_deepmreye_filename( + layout_in, img=img, filetype="report" + ) move_file(deepmreye_mask_report, report_name) - mask_name = create_bidsname(layout_out, img, "mask") - deepmreye_mask_name = get_deepmreye_filename(layout_in, img, "mask") + mask_name = create_bidsname(layout_out, filename=img, filetype="mask") + deepmreye_mask_name = get_deepmreye_filename(layout_in, img=img, filetype="mask") move_file(deepmreye_mask_name, mask_name) - save_sampling_frequency_to_json(layout_out, img) + source = str(Path(img).relative_to(layout_in.root)) + save_sampling_frequency_to_json(layout_out, img=img, source=source) combine_data_with_empty_labels(layout_out, mask_name) -def save_sampling_frequency_to_json(layout_out: BIDSLayout, img: str) -> None: - func_img = nib.load(img) - header = func_img.header - sampling_frequency = header.get_zooms()[3] - if sampling_frequency <= 1: - log.warning(f"Found a repetition time of {sampling_frequency} seconds.") - create_sidecar(layout_out, img, SamplingFrequency=1 / float(sampling_frequency)) - - def prepare_data(cfg: Config) -> None: """Run coregistration and extract data for all subjects. @@ -172,7 +165,12 @@ def prepare_data(cfg: Config) -> None: """ log.info("PREPARING DATA") - layout_in = get_dataset_layout(cfg.input_dir, use_database=True) + layout_in = get_dataset_layout( + cfg.input_dir, + use_database=True, + config=["bids", "derivatives"], + reset_database=cfg.reset_database, + ) check_layout(cfg, layout_in) layout_out = init_dataset(cfg) diff --git a/tests/test_bids_utils.py b/tests/test_bids_utils.py index 8c1217b..97dbe01 100644 --- a/tests/test_bids_utils.py +++ b/tests/test_bids_utils.py @@ -1,14 +1,18 @@ from __future__ import annotations +import json import shutil from pathlib import Path from .utils import pybids_test_dataset +from bidsmreye.bids_utils import check_layout from bidsmreye.bids_utils import create_bidsname from bidsmreye.bids_utils import get_dataset_layout from bidsmreye.bids_utils import init_dataset from bidsmreye.bids_utils import list_subjects from bidsmreye.configuration import Config +from bidsmreye.prepare_data import save_sampling_frequency_to_json +from bidsmreye.utils import set_this_filter def test_create_bidsname(): @@ -67,3 +71,43 @@ def test_list_subjects(): subjects = list_subjects(cfg, layout) assert len(subjects) == 5 + + +def test_save_sampling_frequency_to_json(): + + layout_in = get_dataset_layout(pybids_test_dataset()) + + cfg = Config( + pybids_test_dataset(), + Path(__file__).parent.joinpath("data"), + ) + + this_filter = set_this_filter(cfg, "01", "bold") + + bf = layout_in.get( + return_type="filename", + regex_search=True, + **this_filter, + ) + source = "foo" + save_sampling_frequency_to_json(layout_in, bf[0], source) + sidecar_name = create_bidsname(layout_in, bf[0], "confounds_json") + with open(sidecar_name) as f: + content = json.load(f) + assert content["Sources"][0] == "foo" + + +def test_check_layout_prepare_data(): + + cfg = Config( + pybids_test_dataset(), + Path(__file__).parent.joinpath("data"), + ) + + layout_in = get_dataset_layout( + cfg.input_dir, + use_database=True, + config=["bids", "derivatives"], + reset_database=True, + ) + check_layout(cfg, layout_in) diff --git a/tests/test_utils.py b/tests/test_utils.py index 6f8727e..7796ce6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -93,6 +93,7 @@ def test_set_this_filter_bold(): assert this_filter == { "datatype": "func", + "desc": "preproc", "extension": "nii.*", "run": "1|2", "subject": "001", @@ -123,7 +124,11 @@ def test_set_this_filter_bidsmreye(): def test_set_this_filter_with_bids_filter_file(): bids_filter = { - "eyetrack": {"suffix": "^eyetrack$$", "extension": "tsv", "desc": "preproc"} + "eyetrack": { + "suffix": "^eyetrack$$", + "extension": "tsv", + "desc": "preproc", + } } output_dir = Path().resolve()