diff --git a/docs/source/fl_ready_dataset.md b/docs/source/fl_ready_dataset.md index 32fb394..eb86464 100644 --- a/docs/source/fl_ready_dataset.md +++ b/docs/source/fl_ready_dataset.md @@ -13,11 +13,11 @@ brainflex record fl_ready_dataset start ## Pipeline installation -Install the [Nipoppy-to-Fed-BioMed](https://zenodo.org/records/23988781) conversion pipeline. +Install the [Nipoppy-to-Fed-BioMed](https://zenodo.org/records/23992846) conversion pipeline. This pipeline was written for this FL experiment specifically. ```bash -nipoppy pipeline install 23988781 --study nipoppy_study +nipoppy pipeline install 23992846 --study nipoppy_study ``` The pipeline should now be listed when you run: diff --git a/pipelines/nipoppy2fedbiomed-0.1.0/descriptor.json b/pipelines/nipoppy2fedbiomed-0.1.0/descriptor.json index 0d0d162..377701e 100644 --- a/pipelines/nipoppy2fedbiomed-0.1.0/descriptor.json +++ b/pipelines/nipoppy2fedbiomed-0.1.0/descriptor.json @@ -3,7 +3,7 @@ "description": "Export tabular data from a Nipoppy study for a Fed-BioMed node.", "tool-version": "0.1.0", "schema-version": "0.5", - "command-line": "python [SCRIPT_PATH] [NIPOPPY_ROOT] [OUTPUT_DIR] [PHENOTYPE] [DERIVATIVE] [TAG] [NAME] [DESCRIPTION] [DATASET_PATH] [OUTLIER_THRESHOLD] [HELP]", + "command-line": "python [SCRIPT_PATH] [NIPOPPY_ROOT] [OUTPUT_DIR] [PHENOTYPE] [DERIVATIVE] [TAG] [NAME] [DESCRIPTION] [DATASET_PATH] [OUTLIER_THRESHOLD] [OUTLIER_COLUMNS] [OUTLIER_EXCLUDE_COLUMNS] [HELP]", "inputs": [ { "id": "script_path", @@ -90,11 +90,33 @@ "name": "outlier_threshold", "type": "Number", "value-key": "[OUTLIER_THRESHOLD]", - "description": "Remove rows where any IDP (derivative) value is below Q1 - K*IQR or above Q3 + K*IQR. Default: no outlier removal.", + "description": "Remove rows where any IDP (derivative) value is below Q1 - K*IQR or above Q3 + K*IQR. Columns with zero IQR are ignored. Default: no outlier removal.", "optional": true, "minimum": 0, "command-line-flag": "--outlier-threshold" }, + { + "id": "outlier_columns", + "name": "outlier_columns", + "type": "String", + "value-key": "[OUTLIER_COLUMNS]", + "description": "fnmatch pattern of IDP columns to check for outliers. Can be repeated. Default: all IDP columns.", + "optional": true, + "list": true, + "list-separator": " --outlier-column ", + "command-line-flag": "--outlier-column" + }, + { + "id": "outlier_exclude_columns", + "name": "outlier_exclude_columns", + "type": "String", + "value-key": "[OUTLIER_EXCLUDE_COLUMNS]", + "description": "fnmatch pattern of IDP columns to never check for outliers, even if they match outlier_columns. Can be repeated.", + "optional": true, + "list": true, + "list-separator": " --outlier-exclude-column ", + "command-line-flag": "--outlier-exclude-column" + }, { "id": "help", "name": "help", diff --git a/pipelines/nipoppy2fedbiomed-0.1.0/invocation-age.json b/pipelines/nipoppy2fedbiomed-0.1.0/invocation-age.json index be8eba3..4ea8bf1 100644 --- a/pipelines/nipoppy2fedbiomed-0.1.0/invocation-age.json +++ b/pipelines/nipoppy2fedbiomed-0.1.0/invocation-age.json @@ -13,5 +13,30 @@ ], "name": "age", "dataset_path": "$NIPOPPY_ROOT/derivatives/nipoppy2fedbiomed/0.1.0/output/age/dataset.csv", - "outlier_threshold": 3 + "outlier_threshold": 3, + "outlier_columns": [ + "*_thickness", + "EstimatedTotalIntraCranialVol", + "Left-Lateral-Ventricle", + "Left-Thalamus", + "Left-Caudate", + "Left-Putamen", + "Left-Pallidum", + "Left-Hippocampus", + "Left-Amygdala", + "Left-Accumbens-area", + "Right-Lateral-Ventricle", + "Right-Thalamus", + "Right-Caudate", + "Right-Putamen", + "Right-Pallidum", + "Right-Hippocampus", + "Right-Amygdala", + "Right-Accumbens-area" + ], + "outlier_exclude_columns": [ + "lh_MeanThickness_thickness", + "rh_MeanThickness_thickness", + "rh_temporalpole_thickness" + ] } diff --git a/pipelines/nipoppy2fedbiomed-0.1.0/invocation-sex.json b/pipelines/nipoppy2fedbiomed-0.1.0/invocation-sex.json index d91ed20..789f182 100644 --- a/pipelines/nipoppy2fedbiomed-0.1.0/invocation-sex.json +++ b/pipelines/nipoppy2fedbiomed-0.1.0/invocation-sex.json @@ -13,5 +13,30 @@ ], "name": "sex", "dataset_path": "$NIPOPPY_ROOT/derivatives/nipoppy2fedbiomed/0.1.0/output/sex/dataset.csv", - "outlier_threshold": 3 + "outlier_threshold": 3, + "outlier_columns": [ + "*_thickness", + "EstimatedTotalIntraCranialVol", + "Left-Lateral-Ventricle", + "Left-Thalamus", + "Left-Caudate", + "Left-Putamen", + "Left-Pallidum", + "Left-Hippocampus", + "Left-Amygdala", + "Left-Accumbens-area", + "Right-Lateral-Ventricle", + "Right-Thalamus", + "Right-Caudate", + "Right-Putamen", + "Right-Pallidum", + "Right-Hippocampus", + "Right-Amygdala", + "Right-Accumbens-area" + ], + "outlier_exclude_columns": [ + "lh_MeanThickness_thickness", + "rh_MeanThickness_thickness", + "rh_temporalpole_thickness" + ] } diff --git a/pipelines/nipoppy2fedbiomed-0.1.0/nipoppy2fedbiomed.py b/pipelines/nipoppy2fedbiomed-0.1.0/nipoppy2fedbiomed.py index 9deebb5..b6a4094 100755 --- a/pipelines/nipoppy2fedbiomed-0.1.0/nipoppy2fedbiomed.py +++ b/pipelines/nipoppy2fedbiomed-0.1.0/nipoppy2fedbiomed.py @@ -3,6 +3,7 @@ """Export tabular data from a Nipoppy study for a Fed-BioMed node.""" import argparse +import fnmatch import json import sys from pathlib import Path @@ -100,7 +101,28 @@ def _get_parser() -> argparse.ArgumentParser: metavar="K", help=( "Remove rows where any IDP (derivative) value is below Q1 - K*IQR" - " or above Q3 + K*IQR. Default: no outlier removal." + " or above Q3 + K*IQR. Columns with zero IQR are ignored." + " Default: no outlier removal." + ), + ) + parser.add_argument( + "--outlier-column", + dest="outlier_columns", + action="append", + metavar="PATTERN", + help=( + "fnmatch pattern of IDP columns to check for outliers. Can be repeated." + " Default: all IDP columns." + ), + ) + parser.add_argument( + "--outlier-exclude-column", + dest="outlier_exclude_columns", + action="append", + metavar="PATTERN", + help=( + "fnmatch pattern of IDP columns to never check for outliers," + " even if they match --outlier-column. Can be repeated." ), ) return parser @@ -109,16 +131,44 @@ def _get_parser() -> argparse.ArgumentParser: def _get_outlier_mask( df: "pd.DataFrame", columns: list[str], threshold: float ) -> "pd.Series": - """Return a boolean Series that is True for rows with any outlier value.""" + """Return a boolean Series that is True for rows with any outlier value. + + Columns with zero IQR are ignored. + """ values = df[columns] q1 = values.quantile(0.25) q3 = values.quantile(0.75) iqr = q3 - q1 + nonzero = iqr != 0 + values, q1, q3, iqr = values.loc[:, nonzero], q1[nonzero], q3[nonzero], iqr[nonzero] return ((values < q1 - threshold * iqr) | (values > q3 + threshold * iqr)).any( axis="columns" ) +def _match_columns( + columns: list[str], patterns: list[str] | None, exclude_patterns: list[str] | None +) -> list[str]: + """Return the columns matching any pattern (all if None) and no exclude pattern.""" + + def _matches(column: str, patterns: list[str]) -> bool: + return any(fnmatch.fnmatchcase(column, pattern) for pattern in patterns) + + return [ + column + for column in columns + if (patterns is None or _matches(column, patterns)) + and not _matches(column, exclude_patterns or []) + ] + + +def _get_zero_iqr_columns(df: "pd.DataFrame", columns: list[str]) -> list[str]: + """Return the columns whose IQR is zero.""" + values = df[columns] + iqr = values.quantile(0.75) - values.quantile(0.25) + return [column for column in columns if iqr[column] == 0] + + def nipoppy2fedbiomed( nipoppy_root: Path, output_dir: Path, @@ -129,6 +179,8 @@ def nipoppy2fedbiomed( description: str | None = None, dataset_path: str | None = None, outlier_threshold: float | None = None, + outlier_columns: list[str] | None = None, + outlier_exclude_columns: list[str] | None = None, ) -> tuple[Path, Path]: """Write the CSV dataset and its Fed-BioMed description. @@ -143,6 +195,8 @@ def nipoppy2fedbiomed( ) if not derivatives: raise ValueError("Outlier removal requires at least one derivative") + elif outlier_columns or outlier_exclude_columns: + raise ValueError("Outlier columns require an outlier threshold") nipoppy_root = nipoppy_root.resolve() name = name or nipoppy_root.name if description is None: @@ -164,7 +218,21 @@ def nipoppy2fedbiomed( if n_dropped := n_rows - len(df): print(f"Dropped {n_dropped} rows with missing values") if outlier_threshold is not None: - idp_columns = [column for column in df.columns if column not in phenotypes] + idp_columns = _match_columns( + [column for column in df.columns if column not in phenotypes], + outlier_columns, + outlier_exclude_columns, + ) + if not idp_columns: + raise ValueError( + "No IDP columns match the outlier column patterns" + f" {outlier_columns} (excluding {outlier_exclude_columns})" + ) + if zero_iqr_columns := _get_zero_iqr_columns(df, idp_columns): + print( + f"Skipped {len(zero_iqr_columns)} columns with zero IQR in outlier" + f" removal: {zero_iqr_columns}" + ) n_rows = len(df) df = df.loc[~_get_outlier_mask(df, idp_columns, outlier_threshold)] if df.empty: @@ -213,6 +281,8 @@ def main(argv: list[str] | None = None) -> None: description=args.description, dataset_path=args.dataset_path, outlier_threshold=args.outlier_threshold, + outlier_columns=args.outlier_columns, + outlier_exclude_columns=args.outlier_exclude_columns, ) except (ValueError, FileNotFoundError) as e: sys.exit(f"Error: {e}") diff --git a/tests/pipelines/test_nipoppy2fedbiomed.py b/tests/pipelines/test_nipoppy2fedbiomed.py index 1ddca64..fcff446 100644 --- a/tests/pipelines/test_nipoppy2fedbiomed.py +++ b/tests/pipelines/test_nipoppy2fedbiomed.py @@ -5,6 +5,7 @@ import sys from pathlib import Path from types import ModuleType +from typing import ClassVar import pandas as pd import pytest @@ -112,6 +113,23 @@ def nipoppy_root(tmp_path: Path) -> Path: return nipoppy_root +def _write_thickness(nipoppy_root: Path, values: tuple[float, float, float]) -> None: + """Overwrite the lh_insula thickness values of participants 01, 02 and 03.""" + _write_tsv( + nipoppy_root + / "derivatives" + / "freesurfer" + / FS_VERSION + / "idp" + / "fs_stats-0.2.0" + / f"fs{FS_VERSION}-aparc.DKTatlas-thickness.tsv", + [ + {"participant_id": f"sub-{p}", "session_id": "ses-BL", "lh_insula": v} + for p, v in zip(("01", "02", "03"), values) + ], + ) + + @pytest.fixture def output_dir(tmp_path: Path) -> Path: return tmp_path / "output" @@ -192,6 +210,37 @@ def test_ignores_other_columns(self, module: ModuleType, df: pd.DataFrame): mask = module._get_outlier_mask(df, ["idp1", "idp2"], 3) assert mask.tolist() == [False] * 5 + def test_ignores_zero_iqr_columns(self, module: ModuleType, df: pd.DataFrame): + # Q1 = Q3 = 0: every non-zero value would be outside the fences + df["idp1"] = [0.0, 0.0, 0.0, 0.0, 1.0] + mask = module._get_outlier_mask(df, ["idp1", "idp2"], 3) + assert mask.tolist() == [False] * 5 + + +class TestMatchColumns: + COLUMNS: ClassVar[list[str]] = [ + "lh_insula_thickness", + "rh_insula_thickness", + "Left-Hippocampus", + ] + + def test_all_by_default(self, module: ModuleType): + assert module._match_columns(self.COLUMNS, None, None) == self.COLUMNS + + def test_patterns(self, module: ModuleType): + assert module._match_columns(self.COLUMNS, ["*_thickness"], None) == [ + "lh_insula_thickness", + "rh_insula_thickness", + ] + + def test_exclude_patterns(self, module: ModuleType): + assert module._match_columns( + self.COLUMNS, ["*_thickness", "Left-*"], ["rh_*"] + ) == ["lh_insula_thickness", "Left-Hippocampus"] + + def test_case_sensitive(self, module: ModuleType): + assert module._match_columns(self.COLUMNS, ["left-*"], None) == [] + class TestNipoppy2fedbiomed: def test_nipoppy2fedbiomed( @@ -337,6 +386,104 @@ def test_outlier_removal( assert df["nb:Sex"].tolist() == [1] assert "Dropped 2 rows with outlier IDP values" in capsys.readouterr().out + @pytest.mark.parametrize( + "outlier_columns,outlier_exclude_columns,expected_ages", + [ + # Left-Hippocampus (4000, 4100, 4200): 01 and 03 are outliers + (["Left-*"], None, [30.0]), + # lh_insula (2.6, 2.5, 2.7): 02 and 03 are outliers + (["lh_*"], None, [20.5]), + (["*"], ["lh_*"], [30.0]), + (None, ["Left-*"], [20.5]), + ], + ) + def test_outlier_columns( + self, + module: ModuleType, + nipoppy_root: Path, + output_dir: Path, + outlier_columns: list[str] | None, + outlier_exclude_columns: list[str] | None, + expected_ages: list[float], + ): + _write_thickness(nipoppy_root, (2.6, 2.5, 2.7)) + # with threshold 0, only values within [Q1, Q3] are kept + fpath_csv, _ = module.nipoppy2fedbiomed( + nipoppy_root, + output_dir, + ["nb:Age", "nb:Sex"], + DERIVATIVES, + ["tag"], + outlier_threshold=0, + outlier_columns=outlier_columns, + outlier_exclude_columns=outlier_exclude_columns, + ) + df = pd.read_csv(fpath_csv) + assert df["nb:Age"].tolist() == expected_ages + # excluded columns are still in the output + assert "lh_insula" in df.columns + + def test_outlier_zero_iqr( + self, + module: ModuleType, + nipoppy_root: Path, + output_dir: Path, + capsys: pytest.CaptureFixture, + ): + _write_thickness(nipoppy_root, (2.5, 2.5, 2.5)) + fpath_csv, _ = module.nipoppy2fedbiomed( + nipoppy_root, + output_dir, + ["nb:Age", "nb:Sex"], + DERIVATIVES, + ["tag"], + outlier_threshold=0, + ) + assert pd.read_csv(fpath_csv)["nb:Age"].tolist() == [30.0] + out = capsys.readouterr().out + assert ( + "Skipped 1 columns with zero IQR in outlier removal: ['lh_insula']" in out + ) + assert "Dropped 2 rows with outlier IDP values" in out + + def test_outlier_columns_no_match( + self, module: ModuleType, nipoppy_root: Path, output_dir: Path + ): + with pytest.raises(ValueError, match="No IDP columns match"): + module.nipoppy2fedbiomed( + nipoppy_root, + output_dir, + ["nb:Age"], + DERIVATIVES, + ["tag"], + outlier_threshold=3, + outlier_columns=["Left-*"], + outlier_exclude_columns=["*Hippocampus"], + ) + + @pytest.mark.parametrize( + "outlier_columns,outlier_exclude_columns", + [(["Left-*"], None), (None, ["Left-*"])], + ) + def test_outlier_columns_without_threshold( + self, + module: ModuleType, + nipoppy_root: Path, + output_dir: Path, + outlier_columns: list[str] | None, + outlier_exclude_columns: list[str] | None, + ): + with pytest.raises(ValueError, match="require an outlier threshold"): + module.nipoppy2fedbiomed( + nipoppy_root, + output_dir, + ["nb:Age"], + DERIVATIVES, + ["tag"], + outlier_columns=outlier_columns, + outlier_exclude_columns=outlier_exclude_columns, + ) + def test_outlier_threshold_negative( self, module: ModuleType, nipoppy_root: Path, output_dir: Path ): @@ -397,6 +544,9 @@ def test_main( "--dataset-path", "$NIPOPPY_ROOT/output/dataset.csv", *("--outlier-threshold", "3"), + *("--outlier-column", "Left-*"), + *("--outlier-column", "lh_*"), + *("--outlier-exclude-column", "*_MeanThickness"), ] ) assert "Wrote 3 rows" in capsys.readouterr().out