From d776730f32be08d86c1082c52209fa5d5c3e3fd5 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 8 Sep 2022 15:14:10 +0200 Subject: [PATCH 1/9] Fix typing + rename base classes to concrete + fix some tests --- examples/norun_hcpfc_pearson.py | 4 +- junifer/api/cli.py | 27 +++--- junifer/api/functions.py | 52 +++++++----- junifer/api/registry.py | 9 +- junifer/api/tests/test_functions.py | 8 +- junifer/api/tests/test_registry.py | 11 +-- junifer/data/atlases.py | 83 +++++++++++-------- junifer/data/tests/test_atlases.py | 25 +++--- junifer/datagrabber/__init__.py | 2 +- junifer/datagrabber/datalad_base.py | 3 +- junifer/datagrabber/hcp.py | 24 +++--- ...ern_datalad_base.py => pattern_datalad.py} | 10 +-- junifer/datagrabber/tests/test_base.py | 2 +- ...atalad_base.py => test_pattern_datalad.py} | 4 +- junifer/datareader/default.py | 4 +- junifer/markers/base.py | 23 ++--- junifer/markers/collection.py | 15 +++- junifer/markers/parcel.py | 22 +++-- junifer/markers/pipeline_mixin.py | 2 +- junifer/markers/tests/test_collection.py | 7 +- junifer/markers/tests/test_markers_base.py | 12 +-- junifer/markers/tests/test_pipeline_mixin.py | 6 +- junifer/preprocess/confounds.py | 9 +- junifer/preprocess/tests/test_confounds.py | 24 +++--- junifer/stats.py | 23 ++++- junifer/storage/base.py | 2 +- junifer/storage/sqlite.py | 29 ++++--- junifer/storage/tests/test_sqlite.py | 45 ++++++---- junifer/storage/tests/test_storage_base.py | 4 +- junifer/storage/tests/test_utils.py | 2 +- junifer/storage/utils.py | 4 +- junifer/utils/logging.py | 12 +-- 32 files changed, 302 insertions(+), 207 deletions(-) rename junifer/datagrabber/{pattern_datalad_base.py => pattern_datalad.py} (86%) rename junifer/datagrabber/tests/{test_pattern_datalad_base.py => test_pattern_datalad.py} (97%) diff --git a/examples/norun_hcpfc_pearson.py b/examples/norun_hcpfc_pearson.py index fe2632e92..6875e23b9 100644 --- a/examples/norun_hcpfc_pearson.py +++ b/examples/norun_hcpfc_pearson.py @@ -1,6 +1,8 @@ -""" +"""extraction of FC from HCP data. + HCP FC Extraction ====================== + Authors: Leonard Sasse License: BSD 3 clause """ diff --git a/junifer/api/cli.py b/junifer/api/cli.py index 078b3c4f2..6bcba4392 100644 --- a/junifer/api/cli.py +++ b/junifer/api/cli.py @@ -4,9 +4,10 @@ # Synchon Mandal # License: AGPL -from typing import Dict, List +from typing import Dict, List, Union import click +import pathlib from ..utils.logging import configure_logging, logger, warn_with_log from .functions import collect as api_collect @@ -15,7 +16,7 @@ from .functions import run as api_run from .parser import parse_yaml -def _parse_elements(element: str, config: Dict) -> List: +def _parse_elements(element: str, config: Dict) -> Union[List, None]: """Parse elements from cli. Parameters @@ -58,7 +59,8 @@ def cli() -> None: # pragma: no cover @cli.command() @click.argument( - "filepath", type=click.Path(exists=True, readable=True, dir_okay=False) + "filepath", type=click.Path( + exists=True, readable=True, dir_okay=False, path_type=pathlib.Path) ) @click.option("--element", type=str, multiple=True) @click.option( @@ -80,9 +82,9 @@ def run(filepath: click.Path, element: str, verbose: click.Choice) -> None: The verbosity level: warning, info or debug (default "info"). """ - configure_logging(level=verbose.upper()) + configure_logging(level=str(verbose).upper()) # TODO: add validation - config = parse_yaml(filepath) + config = parse_yaml(filepath) # type: ignore workdir = config["workdir"] datagrabber = config["datagrabber"] markers = config["markers"] @@ -100,8 +102,8 @@ def run(filepath: click.Path, element: str, verbose: click.Choice) -> None: @cli.command() @click.argument( - "filepath", type=click.Path(exists=True, readable=True, dir_okay=False) -) + "filepath", type=click.Path( + exists=True, readable=True, dir_okay=False, path_type=pathlib.Path)) @click.option( "-v", "--verbose", @@ -119,9 +121,9 @@ def collect(filepath: click.Path, verbose: click.Choice) -> None: The verbosity level: warning, info or debug (default "info"). """ - configure_logging(level=verbose.upper()) + configure_logging(level=str(verbose).upper()) # TODO: add validation - config = parse_yaml(filepath) + config = parse_yaml(filepath) # type: ignore storage = config["storage"] # Perform operation api_collect(storage=storage) @@ -129,7 +131,8 @@ def collect(filepath: click.Path, verbose: click.Choice) -> None: @cli.command() @click.argument( - "filepath", type=click.Path(exists=True, readable=True, dir_okay=False) + "filepath", type=click.Path( + exists=True, readable=True, dir_okay=False, path_type=pathlib.Path) ) @click.option("--element", type=str, multiple=True) @click.option("--overwrite", is_flag=True) @@ -163,9 +166,9 @@ def queue( The verbosity level: warning, info or debug (default "info"). """ - configure_logging(level=verbose.upper()) + configure_logging(level=str(verbose).upper()) # TODO: add validation - config = parse_yaml(filepath) + config = parse_yaml(filepath) # type: ignore elements = _parse_elements(element, config) queue_config = config.pop("queue") kind = queue_config.pop("kind") diff --git a/junifer/api/functions.py b/junifer/api/functions.py index 816a2dac0..3c4bc7e4c 100644 --- a/junifer/api/functions.py +++ b/junifer/api/functions.py @@ -8,7 +8,8 @@ import shutil import subprocess from pathlib import Path -from typing import Dict, List, Optional, Sequence, Union +from typing import Dict, List, Optional, Union, Tuple +import typing import yaml @@ -21,7 +22,7 @@ from ..utils.fs import make_executable from .registry import build -def _get_datagrabber(datagrabber_config: Dict) -> Dict: +def _get_datagrabber(datagrabber_config: Dict) -> BaseDataGrabber: """Get datagrabber. Parameters @@ -43,6 +44,7 @@ def _get_datagrabber(datagrabber_config: Dict) -> Dict: baseclass=BaseDataGrabber, init_params=datagrabber_params, ) + datagrabber = typing.cast(BaseDataGrabber, datagrabber) return datagrabber @@ -51,7 +53,7 @@ def run( datagrabber: Dict, markers: List[Dict], storage: Dict, - elements: Union[str, Sequence, None] = None, + elements: Union[str, List[Union[str, Tuple]], Tuple, None] = None, ) -> None: """Run the pipeline on the selected element. @@ -81,8 +83,10 @@ def run( # Convert str to Path if isinstance(workdir, str): workdir = Path(workdir) + if not isinstance(elements, List) and elements is not None: + elements = [elements] # Get datagrabber to use - datagrabber = _get_datagrabber(datagrabber) + datagrabber_object = _get_datagrabber(datagrabber) # Copy to avoid changing the original dict _markers = [x.copy() for x in markers] built_markers = [] @@ -98,22 +102,23 @@ def run( # Get storage engine to use storage_params = storage.copy() storage_kind = storage_params.pop("kind") - storage = build( + storage_object = build( step="storage", name=storage_kind, baseclass=BaseFeatureStorage, init_params=storage_params, ) + storage_object = typing.cast(BaseFeatureStorage, storage_object) # Create new marker collection - mc = MarkerCollection(markers=built_markers, storage=storage) + mc = MarkerCollection(markers=built_markers, storage=storage_object) # Fit elements - with datagrabber: + with datagrabber_object: if elements is not None: for t_element in elements: - mc.fit(datagrabber[t_element]) + mc.fit(datagrabber_object[t_element]) else: - for t_element in datagrabber: - mc.fit(datagrabber[t_element]) + for t_element in datagrabber_object: + mc.fit(datagrabber_object[t_element]) def collect(storage: Dict) -> None: @@ -131,14 +136,15 @@ def collect(storage: Dict) -> None: storage_kind = storage_params.pop("kind") logger.info(f"Collecting data using {storage_kind}") logger.debug(f"\tStorage params: {storage_params}") - storage = build( + storage_object = build( step="storage", name=storage_kind, baseclass=BaseFeatureStorage, init_params=storage_params, ) + storage_object = typing.cast(BaseFeatureStorage, storage_object) logger.debug("Running storage.collect()") - storage.collect() + storage_object.collect() logger.info("Collect done") @@ -147,7 +153,7 @@ def queue( kind: str, jobname: str = "junifer_job", overwrite: bool = False, - elements: Union[str, Sequence, None] = None, + elements: Union[str, List[Union[str, Tuple]], Tuple, None] = None, **kwargs: Union[str, int, bool], ) -> None: # pragma : no cover """Queue a job to be executed later. @@ -209,12 +215,19 @@ def queue( datagrabber = _get_datagrabber(config["datagrabber"]) with datagrabber as dg: elements = dg.get_elements() + + # TODO: Fix typing of elements + if not isinstance(elements, List): + elements = [elements] # type: ignore + + typing.cast(List[Union[str, Tuple]], elements) + if kind == "HTCondor": _queue_condor( jobname=jobname, jobdir=jobdir, yaml_config=yaml_config, - elements=elements, + elements=elements, # type: ignore **kwargs, ) elif kind == "SLURM": @@ -222,7 +235,7 @@ def queue( jobname=jobname, jobdir=jobdir, yaml_config=yaml_config, - elements=elements, + elements=elements, # type: ignore **kwargs, ) else: @@ -235,7 +248,7 @@ def _queue_condor( jobname: str, jobdir: Path, yaml_config: Path, - elements: Union[str, Sequence, None] = None, + elements: List[Union[str, Tuple]], env: Optional[Dict[str, str]] = None, mem: str = "8G", cpus: int = 1, @@ -255,9 +268,8 @@ def _queue_condor( The path to the job directory. yaml_config : pathlib.Path The path to the YAML config file. - elements : str or tuple or list of str or tuple, optional - Element(s) to process. Will be used to index the datagrabber - (default None). + elements : list of str or tuple + Element(s) to process. Will be used to index the datagrabber. env : dict, optional The environment variables passed as dictionary (default None). mem : str, optional @@ -411,7 +423,7 @@ def _queue_slurm( jobname: str, jobdir: Path, yaml_config: Path, - elements: Union[str, Sequence, None] = None, + elements: List[Union[str, Tuple]], ) -> None: """Submit job to SLURM. diff --git a/junifer/api/registry.py b/junifer/api/registry.py index e3f0d42e7..1d567a82a 100644 --- a/junifer/api/registry.py +++ b/junifer/api/registry.py @@ -5,10 +5,13 @@ # Synchon Mandal # License: AGPL -from typing import Dict, List, Optional +from typing import TYPE_CHECKING, Dict, List, Optional, Union from ..utils.logging import logger, raise_error - +if TYPE_CHECKING: + from ..datagrabber.base import BaseDataGrabber + from ..markers.base import PipelineStepMixin + from ..storage.base import BaseFeatureStorage # Define valid steps for operation _valid_steps = [ @@ -96,7 +99,7 @@ def build( name: str, baseclass: type, init_params: Optional[Dict] = None, -) -> type: +) -> Union["BaseDataGrabber", "PipelineStepMixin", "BaseFeatureStorage"]: """Ensure that the given object is an instance of the given class. Parameters diff --git a/junifer/api/tests/test_functions.py b/junifer/api/tests/test_functions.py index 57f1cf7d6..3b175c901 100644 --- a/junifer/api/tests/test_functions.py +++ b/junifer/api/tests/test_functions.py @@ -59,7 +59,7 @@ def test_run_single_element(tmp_path: Path) -> None: outdir.mkdir() # Create storage uri = outdir / "test.db" - storage["uri"] = uri + storage["uri"] = uri # type: ignore # Run operations run( workdir=workdir, @@ -90,7 +90,7 @@ def test_run_multi_element(tmp_path: Path) -> None: outdir.mkdir() # Create storage uri = outdir / "test.db" - storage["uri"] = uri + storage["uri"] = uri # type: ignore # Run operations run( workdir=workdir, @@ -121,7 +121,7 @@ def test_run_and_collect(tmp_path: Path) -> None: outdir.mkdir() # Create storage uri = outdir / "test.db" - storage["uri"] = uri + storage["uri"] = uri # type: ignore # Run operations run( workdir=workdir, @@ -133,7 +133,7 @@ def test_run_and_collect(tmp_path: Path) -> None: dg = build( step="datagrabber", name=datagrabber["kind"], baseclass=BaseDataGrabber ) - elements = dg.get_elements() + elements = dg.get_elements() # type: ignore # This should create 10 files files = list(outdir.glob("*.db")) assert len(files) == len(elements) diff --git a/junifer/api/tests/test_registry.py b/junifer/api/tests/test_registry.py index ff4b9264c..7ac9a88ca 100644 --- a/junifer/api/tests/test_registry.py +++ b/junifer/api/tests/test_registry.py @@ -4,6 +4,7 @@ # Leonard Sasse # Synchon Mandal # License: AGPL +from typing import Type import logging from abc import ABC @@ -18,7 +19,7 @@ from junifer.storage import SQLiteFeatureStorage def test_register_invalid_step(): """Test register invalid step name.""" with pytest.raises(ValueError, match="Invalid step:"): - register(step="foo", name="bar", klass="baz") + register(step="foo", name="bar", klass=str) # TODO: improve parametrization @@ -30,7 +31,7 @@ def test_register_invalid_step(): ], ) def test_register( - caplog: pytest.LogCaptureFixture, step: str, name: str, klass: str + caplog: pytest.LogCaptureFixture, step: str, name: str, klass: Type ) -> None: """Test register. @@ -70,7 +71,7 @@ def test_get_step_names_absent() -> None: def test_get_step_names() -> None: """Test get step names.""" # Register datagrabber - register(step="datagrabber", name="bar", klass="baz") + register(step="datagrabber", name="bar", klass=str) # Get step names for datagrabber datagrabbers = get_step_names(step="datagrabber") # Check for datagrabber step name @@ -93,10 +94,10 @@ def test_get_class_invalid_name() -> None: def test_get_class(): """Test get class.""" # Register datagrabber - register(step="datagrabber", name="bar", klass="baz") + register(step="datagrabber", name="bar", klass=str) # Get class obj = get_class(step="datagrabber", name="bar") - assert obj == "baz" + assert obj == str # TODO: possible parametrization? diff --git a/junifer/data/atlases.py b/junifer/data/atlases.py index 7fe70a46a..d2aeeb0f6 100644 --- a/junifer/data/atlases.py +++ b/junifer/data/atlases.py @@ -10,7 +10,7 @@ import shutil import tempfile import zipfile from pathlib import Path -from typing import TYPE_CHECKING, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, List, Optional, Tuple, Union, Any, Dict import nibabel as nib import numpy as np @@ -37,10 +37,11 @@ Optional keys: """ # TODO: have separate dictionary for built-in -_available_atlases = { +_available_atlases: Dict[str, Dict[Any, Any]] = { "SUITxSUIT": {"family": "SUIT", "space": "SUIT"}, "SUITxMNI": {"family": "SUIT", "space": "MNI"}, } + # Add Schaefer atlas info for n_rois in range(100, 1001, 100): for t_net in [7, 17]: @@ -150,7 +151,7 @@ def list_atlases() -> List[str]: def load_atlas( name: str, atlas_dir: Union[str, Path, None] = None, - resolution: Optional[int] = None, + resolution: Optional[float] = None, path_only: bool = False, ) -> Tuple[Optional["Nifti1Image"], List[str], Path]: """Load a brain atlas (including a label file). @@ -165,7 +166,7 @@ def load_atlas( atlas_dir : str or pathlib.Path, optional Path where the atlas files are stored. The default location is "$HOME/junifer/data/atlas" (default None). - resolution : int, optional + resolution : float, optional The desired resolution of the atlas to load. If it is not available, the closest resolution will be loaded. Preferably, use a resolution higher than the desired one. By default, will load the highest one @@ -238,7 +239,7 @@ def load_atlas( def _retrieve_atlas( family: str, atlas_dir: Union[str, Path, None] = None, - resolution: Optional[int] = None, + resolution: Optional[float] = None, **kwargs, ) -> Tuple[Path, List[str]]: """Retrieve a brain atlas object from nilearn or a specified online source. @@ -254,7 +255,7 @@ def _retrieve_atlas( atlas_dir : str or pathlib.Path, optional Path where the retrieved atlas file is stored. The default location is "$HOME/junifer/data/atlas" (default None). - resolution : int, optional + resolution : float, optional The desired resolution of the atlas to load. If it is not available, the closest resolution will be loaded. Preferably, use a resolution higher than the desired one. By default, will load the highest one @@ -326,21 +327,21 @@ def _retrieve_atlas( def _closest_resolution( - resolution: int, - valid_resolution: Union[List[int], np.ndarray], -) -> int: + resolution: Optional[float], + valid_resolution: Union[List[float], List[int], np.ndarray], +) -> Union[float, int]: """Find the closest resolution. Parameters ---------- - resolution : int + resolution : float The given resolution. - valid_resolution : list of int or np.ndarray + valid_resolution : list of float or np.ndarray The array of valid resolutions. Returns ------- - int + float The closest valid resolution. """ @@ -363,7 +364,7 @@ def _closest_resolution( def _retrieve_schaefer( atlas_dir: Path, - resolution: int, + resolution: Optional[float] = None, n_rois: Optional[int] = None, yeo_networks: int = 7, ) -> Tuple[Path, List[str]]: @@ -373,8 +374,11 @@ def _retrieve_schaefer( ---------- atlas_dir : pathlib.Path The path to the atlas data directory. - resolution : {1, 2} - The resolution of the atlas to load. + resolution : float, optional + The desired resolution of the atlas to load. If it is not available, + the closest resolution will be loaded. Preferably, use a resolution + higher than the desired one. By default, will load the highest one + (default None). Available resolutions for this atlas are 1mm and 2mm. n_rois : {100, 200, 300, 400, 500, 600, 700, 800, 900, 1000}, optional Granularity of the atlas to be used (default None). yeo_networks : {7, 17}, optional @@ -440,7 +444,7 @@ def _retrieve_schaefer( datasets.fetch_atlas_schaefer_2018( n_rois=n_rois, yeo_networks=yeo_networks, - resolution_mm=resolution, + resolution_mm=resolution, # type: ignore we know it's 1 or 2 data_dir=str(atlas_dir.absolute()), ) @@ -462,7 +466,7 @@ def _retrieve_schaefer( def _retrieve_tian( atlas_dir: Path, - resolution: int, + resolution: Optional[float] = None, scale: Optional[int] = None, space: str = "MNI6thgeneration", magneticfield: str = "3T", @@ -473,8 +477,12 @@ def _retrieve_tian( ---------- atlas_dir : pathlib.Path The path to the atlas data directory. - resolution : {1, 2} - The resolution of the atlas to load. + resolution : float, optional + The desired resolution of the atlas to load. If it is not available, + the closest resolution will be loaded. Preferably, use a resolution + higher than the desired one. By default, will load the highest one + (default None). Available resolutions for this atlas depend on the + space and magnetic field. scale : {1, 2, 3, 4}, optional Scale of atlas (defines granularity) (default None). space : {"MNI6thgeneration", "MNInonlinear2009cAsym"}, optional @@ -505,32 +513,34 @@ def _retrieve_tian( logger.info(f"\tresolution: {resolution}") # check validity of atlas parameters _valid_scales = [1, 2, 3, 4] - _valid_fields = ["3T", "7T"] if scale not in _valid_scales: raise_error( f"The parameter `scale` ({scale}) needs to be one of the " f"following: {_valid_scales}" ) - if magneticfield not in _valid_fields: - raise_error( - f"The parameter `magneticfield` ({magneticfield}) needs to be " - f"one of the following: {_valid_fields}" - ) + _valid_resolutions = [] # avoid pylance error if magneticfield == "3T": _valid_spaces = ["MNI6thgeneration", "MNInonlinear2009cAsym"] if space == "MNI6thgeneration": _valid_resolutions = [1, 2] elif space == "MNInonlinear2009cAsym": _valid_resolutions = [2] + else: + raise_error( + f"The parameter `space` ({space}) for 3T needs to be one of " + f"the following: {_valid_spaces}" + ) elif magneticfield == "7T": - _valid_spaces = ["MNI6thgeneration"] _valid_resolutions = [1.6] - - if space not in _valid_spaces: + if space != "MNI6thgeneration": + raise_error( + f"The parameter `space` ({space}) for 7T needs to be " + f"MNI6thgeneration") + else: raise_error( - f"The parameter `space` ({space}) needs to be one of " - f"the following: {_valid_spaces}" + f"The parameter `magneticfield` ({magneticfield}) needs to be " + f"one of the following: 3T or 7T" ) resolution = _closest_resolution(resolution, _valid_resolutions) @@ -581,6 +591,8 @@ def _retrieve_tian( "Currently there are no labels provided for the 7T Tian atlas. " "A simple numbering scheme for distinction was therefore used." ) + else: # pragma: no cover + raise_error('This should not happen. Please report this error.') # check existence of atlas if not (atlas_fname.exists() and atlas_lname.exists()): @@ -613,7 +625,9 @@ def _retrieve_tian( def _retrieve_suit( - atlas_dir: Path, resolution: int, space: str = "MNI" + atlas_dir: Path, + resolution: Optional[float], + space: str = "MNI" ) -> Tuple[Path, List[str]]: """Retrieve SUIT atlas. @@ -621,8 +635,11 @@ def _retrieve_suit( ---------- atlas_dir : pathlib.Path The path to the atlas data directory. - resolution : {1, 2} - The resolution of the atlas to load. + resolution : float, optional + The desired resolution of the atlas to load. If it is not available, + the closest resolution will be loaded. Preferably, use a resolution + higher than the desired one. By default, will load the highest one + (default None). Available resolutions for this atlas are 1mm and 2mm. space : {"MNI", "SUIT"}, optional Space of atlas (default "MNI"). (For more information see http://www.diedrichsenlab.org/imaging/suit.htm). diff --git a/junifer/data/tests/test_atlases.py b/junifer/data/tests/test_atlases.py index 6b9eea365..df70002fb 100644 --- a/junifer/data/tests/test_atlases.py +++ b/junifer/data/tests/test_atlases.py @@ -166,7 +166,7 @@ def test_schaefer_atlas(tmp_path: Path) -> None: assert img is not None assert fname.name == fname1 assert len(lbl) == 100 - assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) # type: ignore # Test with Path img, lbl, fname = load_atlas(name="Schaefer100x7", atlas_dir=tmp_path) @@ -180,7 +180,7 @@ def test_schaefer_atlas(tmp_path: Path) -> None: assert fname.name == fname2 assert len(lbl) == 100 assert img2 is not None - assert_array_equal(img2.header["pixdim"][1:4], [2, 2, 2]) + assert_array_equal(img2.header["pixdim"][1:4], [2, 2, 2]) # type: ignore # Load atlas img2, lbl, fname = load_atlas( "Schaefer100x7", @@ -191,7 +191,7 @@ def test_schaefer_atlas(tmp_path: Path) -> None: assert fname.name == fname2 assert len(lbl) == 100 assert img2 is not None - assert_array_equal(img2.header["pixdim"][1:4], [2, 2, 2]) + assert_array_equal(img2.header["pixdim"][1:4], [2, 2, 2]) # type: ignore # Load atlas img2, lbl, fname = load_atlas( "Schaefer100x7", @@ -202,7 +202,7 @@ def test_schaefer_atlas(tmp_path: Path) -> None: assert fname.name == fname1 assert len(lbl) == 100 assert img2 is not None - assert_array_equal(img2.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img2.header["pixdim"][1:4], [1, 1, 1]) # type: ignore # Load atlas img2, lbl, fname = load_atlas( "Schaefer100x7", @@ -213,7 +213,7 @@ def test_schaefer_atlas(tmp_path: Path) -> None: assert fname.name == fname1 assert len(lbl) == 100 assert img2 is not None - assert_array_equal(img2.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img2.header["pixdim"][1:4], [1, 1, 1]) # type: ignore def test_load_atlas_schaefer() -> None: @@ -274,7 +274,7 @@ def test_suit(tmp_path: Path) -> None: assert img is not None assert fname.name == fname1 assert len(lbl) == 34 - assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) # type: ignore # Load atlas img, lbl, fname = load_atlas(name="SUITxSUIT", atlas_dir=tmp_path) @@ -282,7 +282,7 @@ def test_suit(tmp_path: Path) -> None: assert img is not None assert fname.name == fname1 assert len(lbl) == 34 - assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) # type: ignore # Load atlas img, lbl, fname = load_atlas(name="SUITxMNI", atlas_dir=tmp_path) @@ -290,7 +290,7 @@ def test_suit(tmp_path: Path) -> None: assert img is not None assert fname.name == fname1 assert len(lbl) == 34 - assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) # type: ignore def test_retrieve_suit_incorrect_space(tmp_path: Path) -> None: @@ -346,7 +346,7 @@ def test_tian_3T_6thgeneration( assert img is not None assert fname.name == fname1 assert len(lbl) == n_label - assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) + assert_array_equal(img.header["pixdim"][1:4], [1, 1, 1]) # type: ignore # Load atlas img, lbl, fname = load_atlas( name=f"TianxS{scale}x3TxMNI6thgeneration", @@ -357,7 +357,7 @@ def test_tian_3T_6thgeneration( assert img is not None assert fname.name == fname1 assert len(lbl) == n_label - assert_array_equal(img.header["pixdim"][1:4], [2, 2, 2]) + assert_array_equal(img.header["pixdim"][1:4], [2, 2, 2]) # type: ignore @pytest.mark.parametrize( @@ -400,7 +400,7 @@ def test_tian_3T_nonlinear2009cAsym( assert img is not None assert fname.name == fname1 assert len(lbl) == n_label - assert_array_equal(img.header["pixdim"][1:4], [2, 2, 2]) + assert_array_equal(img.header["pixdim"][1:4], [2, 2, 2]) # type: ignore @pytest.mark.parametrize( @@ -442,7 +442,8 @@ def test_tian_7T_6thgeneration( assert img is not None assert fname.name == fname1 assert len(lbl) == n_label - assert_array_almost_equal(img.header["pixdim"][1:4], [1.6, 1.6, 1.6]) + assert_array_almost_equal( + img.header["pixdim"][1:4], [1.6, 1.6, 1.6]) # type: ignore def test_retrieve_tian_incorrect_space(tmp_path: Path) -> None: diff --git a/junifer/datagrabber/__init__.py b/junifer/datagrabber/__init__.py index 8f0872fce..5975d9b5d 100644 --- a/junifer/datagrabber/__init__.py +++ b/junifer/datagrabber/__init__.py @@ -9,4 +9,4 @@ from .datalad_base import DataladDataGrabber from .hcp import DataladHCP1200, HCP1200 from .multiple import MultipleDataGrabber from .pattern import PatternDataGrabber -from .pattern_datalad_base import PatternDataladDataGrabber +from .pattern_datalad import PatternDataladDataGrabber diff --git a/junifer/datagrabber/datalad_base.py b/junifer/datagrabber/datalad_base.py index 28cc2c6d1..54664a1ff 100644 --- a/junifer/datagrabber/datalad_base.py +++ b/junifer/datagrabber/datalad_base.py @@ -120,7 +120,8 @@ class DataladDataGrabber(BaseDataGrabber): def install(self) -> None: """Install the datalad dataset into the datadir.""" logger.debug(f"Installing dataset {self.uri} to {self._datadir}") - self._dataset = dl.install(self._datadir, source=self.uri) + self._dataset = dl.install( # type: ignore because of datalad + self._datadir, source=self.uri) logger.debug("Dataset installed") def remove(self): diff --git a/junifer/datagrabber/hcp.py b/junifer/datagrabber/hcp.py index 1ebfc0a73..9db59031b 100644 --- a/junifer/datagrabber/hcp.py +++ b/junifer/datagrabber/hcp.py @@ -55,18 +55,19 @@ class HCP1200(PatternDataGrabber): ] # Set default tasks if tasks is None: - self.tasks = all_tasks + self.tasks: List[str] = all_tasks # Convert single task into list - if isinstance(tasks, str): - tasks = [tasks] - # Check for invalid task(s) - for task in tasks: - if task not in all_tasks: - raise_error( - f"'{task}' is not a valid HCP-YA fMRI task input. " - f"Valid task values can be any or all of {all_tasks}." - ) - + else: + if not isinstance(tasks, List): + tasks = [tasks] + # Check for invalid task(s) + for task in tasks: + if task not in all_tasks: + raise_error( + f"'{task}' is not a valid HCP-YA fMRI task input. " + f"Valid task values can be any or all of {all_tasks}." + ) + self.tasks: List[str] = tasks # All phase encodings all_phase_encodings = ["LR", "RL"] # Set phase encodings @@ -102,7 +103,6 @@ class HCP1200(PatternDataGrabber): patterns=patterns, replacements=replacements, ) - self.tasks = tasks self.phase_encodings = phase_encodings def __getitem__(self, element: Tuple[str, str, str]) -> Dict[str, Path]: diff --git a/junifer/datagrabber/pattern_datalad_base.py b/junifer/datagrabber/pattern_datalad.py similarity index 86% rename from junifer/datagrabber/pattern_datalad_base.py rename to junifer/datagrabber/pattern_datalad.py index 5a43e6ec6..07e5e296d 100644 --- a/junifer/datagrabber/pattern_datalad_base.py +++ b/junifer/datagrabber/pattern_datalad.py @@ -5,7 +5,7 @@ # Synchon Mandal # License: AGPL -from typing import Dict, List, Optional +from typing import Dict, List from ..api.decorators import register_datagrabber from .datalad_base import DataladDataGrabber @@ -22,8 +22,8 @@ class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): Parameters ---------- - types : list of str, optional - The types of data to be grabbed (default None). + types : list of str + The types of data to be grabbed. patterns : dict, optional Patterns for each type of data as a dictionary. The keys are the types and the values are the patterns. Each occurrence of the string @@ -41,8 +41,8 @@ class PatternDataladDataGrabber(DataladDataGrabber, PatternDataGrabber): def __init__( self, - types: Optional[List[str]] = None, - patterns: Optional[Dict[str, str]] = None, + types: List[str], + patterns: Dict[str, str], **kwargs, ) -> None: """Initialize the class.""" diff --git a/junifer/datagrabber/tests/test_base.py b/junifer/datagrabber/tests/test_base.py index ba0caae61..91d622d62 100644 --- a/junifer/datagrabber/tests/test_base.py +++ b/junifer/datagrabber/tests/test_base.py @@ -15,7 +15,7 @@ from junifer.datagrabber.base import BaseDataGrabber def test_BaseDataGrabber_abstractness() -> None: """Test BaseDataGrabber is abstract base class.""" with pytest.raises(TypeError, match=r"abstract"): - BaseDataGrabber(datadir="/tmp", types=["func"]) + BaseDataGrabber(datadir="/tmp", types=["func"]) # type: ignore def test_BaseDataGrabber() -> None: diff --git a/junifer/datagrabber/tests/test_pattern_datalad_base.py b/junifer/datagrabber/tests/test_pattern_datalad.py similarity index 97% rename from junifer/datagrabber/tests/test_pattern_datalad_base.py rename to junifer/datagrabber/tests/test_pattern_datalad.py index 0d445c9e8..105918c44 100644 --- a/junifer/datagrabber/tests/test_pattern_datalad_base.py +++ b/junifer/datagrabber/tests/test_pattern_datalad.py @@ -1,4 +1,4 @@ -"""Provide tests for pattern_datalad_base.""" +"""Provide tests for pattern_datalad.""" # Authors: Federico Raimondo # Leonard Sasse @@ -9,7 +9,7 @@ from pathlib import Path import pytest -from junifer.datagrabber.pattern_datalad_base import PatternDataladDataGrabber +from junifer.datagrabber.pattern_datalad import PatternDataladDataGrabber _testing_dataset = { diff --git a/junifer/datareader/default.py b/junifer/datareader/default.py index 34996f6de..7bddd2cc1 100644 --- a/junifer/datareader/default.py +++ b/junifer/datareader/default.py @@ -5,7 +5,7 @@ # License: AGPL from pathlib import Path -from typing import Dict +from typing import Dict, List import nibabel as nib import pandas as pd @@ -33,7 +33,7 @@ class DefaultDataReader(PipelineStepMixin): """Mixin class for default data reader.""" # TODO: complete type annotations - def validate_input(self, input): + def validate_input(self, input: List[str]) -> None: """Validate input. Parameters diff --git a/junifer/markers/base.py b/junifer/markers/base.py index 1fef738bf..d232f1405 100644 --- a/junifer/markers/base.py +++ b/junifer/markers/base.py @@ -4,7 +4,7 @@ # Synchon Mandal # License: AGPL -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union from ..utils import logger, raise_error from .pipeline_mixin import PipelineStepMixin @@ -22,7 +22,11 @@ class BaseMarker(PipelineStepMixin): """ - def __init__(self, on: List, name: Optional[str] = None) -> None: + def __init__( + self, + on: Union[List[str], str], + name: Optional[str] = None + ) -> None: """Initialize the class.""" if not isinstance(on, list): on = [on] @@ -78,24 +82,23 @@ class BaseMarker(PipelineStepMixin): Parameters ---------- input : list of str - The input to the pipeline step. The list must contain the + The input to the marker. The list must contain the available Junifer Data dictionary keys. Returns ------- list of str - The updated list of available Junifer Data dictionary keys after - the pipeline step. + The updated list of output kinds, as storage possibilities. """ - pass + raise_error(msg="compute() not implemented", klass=NotImplementedError) - def compute(self, input: List[str]) -> Dict: + def compute(self, input: Dict) -> Dict: """Compute. Parameters ---------- - input : list of str + input : Dict[str, Dict] The input to the pipeline step. The list must contain the available Junifer Data dictionary keys. @@ -108,7 +111,7 @@ class BaseMarker(PipelineStepMixin): raise_error(msg="compute() not implemented", klass=NotImplementedError) # TODO: complete type annotations - def store(self, input: List[str], out: Dict, storage) -> None: + def store(self, kind: str, out: Dict, storage) -> None: """Store. Parameters @@ -120,7 +123,7 @@ class BaseMarker(PipelineStepMixin): raise_error(msg="store() not implemented", klass=NotImplementedError) # TODO: complete type annotations - def fit_transform(self, input: List[str], storage=None) -> Dict: + def fit_transform(self, input: Dict[str, Dict], storage=None) -> Dict: """Fit and transform. Parameters diff --git a/junifer/markers/collection.py b/junifer/markers/collection.py index f658e1b9f..98fc57599 100644 --- a/junifer/markers/collection.py +++ b/junifer/markers/collection.py @@ -5,10 +5,14 @@ # License: AGPL from collections import Counter -from typing import Dict, Optional +from typing import Dict, Optional, List + +from junifer.markers.pipeline_mixin import PipelineStepMixin from ..datareader import DefaultDataReader from ..utils import logger +from .base import BaseMarker +from ..storage.base import BaseFeatureStorage class MarkerCollection: @@ -24,7 +28,11 @@ class MarkerCollection: """ def __init__( - self, markers, datareader=None, preprocessing=None, storage=None + self, + markers: List[BaseMarker], + datareader: Optional[PipelineStepMixin] = None, + preprocessing: Optional[PipelineStepMixin] = None, + storage: Optional[BaseFeatureStorage] = None ): """Initialize the class.""" # Check that the markers have different names @@ -42,8 +50,7 @@ class MarkerCollection: self._preprocessing = preprocessing self._storage = storage - # TODO: complete type annotations - def fit(self, input) -> Optional[Dict]: + def fit(self, input: Dict[str, Dict]) -> Optional[Dict]: """Fit the pipeline. Parameters diff --git a/junifer/markers/parcel.py b/junifer/markers/parcel.py index 6b217d2c5..755c12f15 100644 --- a/junifer/markers/parcel.py +++ b/junifer/markers/parcel.py @@ -42,7 +42,7 @@ class ParcelAggregation(BaseMarker): on = ["T1w", "BOLD", "VBM_GM", "VBM_WM", "fALFF", "GCOR", "LCOR"] super().__init__(on=on, name=name) - def get_output_kind(self, input: List[str]) -> str: + def get_output_kind(self, input: List[str]) -> List[str]: """Get output kind. Parameters @@ -56,13 +56,18 @@ class ParcelAggregation(BaseMarker): The kind of output. """ - if input in ["VBM_GM", "VBM_WM", "fALFF", "GCOR", "LCOR"]: - return "table" - if input in ["BOLD"]: - return "timeseries" + outputs = [] + for t_input in input: + if t_input in ["VBM_GM", "VBM_WM", "fALFF", "GCOR", "LCOR"]: + outputs.append("table") + elif input in ["BOLD"]: + outputs.append("timeseries") + else: + raise ValueError(f"Unknown input kind for {t_input}") + return outputs # TODO: complete type annotations - def store(self, kind: List[str], out, storage) -> None: + def store(self, kind: str, out, storage) -> None: """Store. Parameters @@ -98,7 +103,7 @@ class ParcelAggregation(BaseMarker): self.method, func_params=self.method_params ) # Get the min of the voxels sizes and use it as the resolution - resolution = np.min(t_input.header.get_zooms()[:3]) + resolution = np.min(t_input.header.get_zooms()[:3]) # type: ignore t_atlas, t_labels, _ = load_atlas(self.atlas, resolution=resolution) atlas_img_res = resample_to_img( t_atlas, @@ -110,7 +115,8 @@ class ParcelAggregation(BaseMarker): img=atlas_img_res, ) logger.debug("Masking") - masker = NiftiMasker(atlas_bin, target_affine=t_input.affine) + masker = NiftiMasker( + atlas_bin, target_affine=t_input.affine) # type: ignore # Mask the input data and the atlas data = masker.fit_transform(t_input) diff --git a/junifer/markers/pipeline_mixin.py b/junifer/markers/pipeline_mixin.py index 22cd52da4..915458810 100644 --- a/junifer/markers/pipeline_mixin.py +++ b/junifer/markers/pipeline_mixin.py @@ -91,7 +91,7 @@ class PipelineStepMixin: self.validate_input(input=input) return self.get_output_kind(input=input) - def fit_transform(self, input: List[str]) -> None: + def fit_transform(self, input: Dict[str, Dict]) -> Dict[str, Dict]: """Fit and transform. Parameters diff --git a/junifer/markers/tests/test_collection.py b/junifer/markers/tests/test_collection.py index 4645ab7ae..224bc6f04 100644 --- a/junifer/markers/tests/test_collection.py +++ b/junifer/markers/tests/test_collection.py @@ -86,11 +86,13 @@ def test_marker_collection(): with dg: input = dg["sub-01"] out2 = mc2.fit(input) + assert out2 is not None for t_marker in markers: t_name = t_marker.name assert_array_equal( - out[t_name]["VBM_GM"]["data"], out2[t_name]["VBM_GM"]["data"] - ) # type: ignore + out[t_name]["VBM_GM"]["data"], + out2[t_name]["VBM_GM"]["data"] + ) def test_MarkerCollection_storage(tmp_path) -> None: @@ -125,6 +127,7 @@ def test_MarkerCollection_storage(tmp_path) -> None: markers=markers, storage=storage, datareader=DefaultDataReader() ) mc.validate(dg) + assert mc._storage is not None assert mc._storage.uri == storage.uri with dg: input = dg["sub-01"] diff --git a/junifer/markers/tests/test_markers_base.py b/junifer/markers/tests/test_markers_base.py index f91ac3eb0..c6459d625 100644 --- a/junifer/markers/tests/test_markers_base.py +++ b/junifer/markers/tests/test_markers_base.py @@ -51,19 +51,19 @@ def test_BaseMarker() -> None: """Test base class.""" base = BaseMarker(on=["bold", "dwi"], name="mymarker") input_ = {"bold": {"path": "test"}, "t2": {"path": "test"}} - base.validate_input(input_) + base.validate_input(list(input_.keys())) wrong_input = {"t2": {"path": "test"}} with pytest.raises(ValueError): - base.validate_input(wrong_input) + base.validate_input(list(wrong_input.keys())) - output = base.get_output_kind(input_) - assert output is None + with pytest.raises(NotImplementedError): + base.get_output_kind(list(wrong_input.keys())) with pytest.raises(NotImplementedError): base.fit_transform(input_) - base.compute = lambda x: {"data": 1} + base.compute = lambda x: {"data": 1} # type: ignore out = base.fit_transform(input_) assert out["bold"]["data"] == 1 @@ -71,7 +71,7 @@ def test_BaseMarker() -> None: assert out["bold"]["meta"]["marker"]["class"] == "BaseMarker" base2 = BaseMarker(on="bold", name="mymarker") - base2.compute = lambda x: {"data": 1} + base2.compute = lambda x: {"data": 1} # type: ignore out2 = base2.fit_transform(input_) assert out2["bold"]["data"] == 1 assert out2["bold"]["meta"]["marker"]["name"] == "bold_mymarker" diff --git a/junifer/markers/tests/test_pipeline_mixin.py b/junifer/markers/tests/test_pipeline_mixin.py index ace380e9a..7086a7805 100644 --- a/junifer/markers/tests/test_pipeline_mixin.py +++ b/junifer/markers/tests/test_pipeline_mixin.py @@ -13,11 +13,11 @@ def test_PipelineStepMixin() -> None: """Test PipelineStepMixin.""" mixin = PipelineStepMixin() with pytest.raises(NotImplementedError): - mixin.validate_input(None) + mixin.validate_input([]) with pytest.raises(NotImplementedError): - mixin.get_output_kind(None) + mixin.get_output_kind([]) with pytest.raises(NotImplementedError): - mixin.fit_transform(None) + mixin.fit_transform({}) def test_pipeline_step_mixin_meta(): diff --git a/junifer/preprocess/confounds.py b/junifer/preprocess/confounds.py index 869097b08..e8f413947 100644 --- a/junifer/preprocess/confounds.py +++ b/junifer/preprocess/confounds.py @@ -5,7 +5,7 @@ # Synchon Mandal # License: AGPL -from typing import TYPE_CHECKING, Dict, List, Optional +from typing import TYPE_CHECKING, Dict, List, Optional, Union import numpy as np import pandas as pd @@ -18,7 +18,7 @@ from ..utils import logger, raise_error if TYPE_CHECKING: - from nibabel import Nifti1Image + from nibabel import Nifti1Image, Nifti2Image, MGHImage class BaseConfoundRemover(PipelineStepMixin): @@ -218,7 +218,8 @@ class BaseConfoundRemover(PipelineStepMixin): def _remove_confounds( self, bold_img: "Nifti1Image", confounds_df: pd.DataFrame - ) -> "Nifti1Image": + ) -> Union["Nifti1Image", "Nifti2Image", "MGHImage", List]: + """Remove confounds from the BOLD data.""" """Remove confounds from the BOLD image. Parameters @@ -242,7 +243,7 @@ class BaseConfoundRemover(PipelineStepMixin): t_r = self.t_r if t_r is None: logger.info("No `t_r` specified, using t_r from nifti header") - zooms = bold_img.header.get_zooms() + zooms = bold_img.header.get_zooms() # type: ignore t_r = zooms[3] logger.info( f"Read t_r from nifti header: {t_r}", diff --git a/junifer/preprocess/tests/test_confounds.py b/junifer/preprocess/tests/test_confounds.py index 01463cdd9..8050e1d2d 100644 --- a/junifer/preprocess/tests/test_confounds.py +++ b/junifer/preprocess/tests/test_confounds.py @@ -140,8 +140,8 @@ def test_baseconfoundremover() -> None: cr = BaseConfoundRemover( strategy=strat1, spike=None, mask_img=simsk, t_r=0.75 ) - cr.validate_input(input_data_obj.keys()) - out_type = cr.get_output_kind(input_data_obj.keys()) + cr.validate_input(list(input_data_obj.keys())) + out_type = cr.get_output_kind(list(input_data_obj.keys())) assert "BOLD" in out_type @@ -175,8 +175,8 @@ def test_baseconfoundremover() -> None: cr = BaseConfoundRemover( strategy=strat2, spike=None, mask_img=simsk, t_r=0.75 ) - cr.validate_input(input_data_obj.keys()) - out_type = cr.get_output_kind(input_data_obj.keys()) + cr.validate_input(list(input_data_obj.keys())) + out_type = cr.get_output_kind(list(input_data_obj.keys())) assert "BOLD" in out_type @@ -207,8 +207,8 @@ def test_baseconfoundremover() -> None: cr = BaseConfoundRemover( strategy=strat3, spike=None, mask_img=simsk, t_r=0.75 ) - cr.validate_input(input_data_obj.keys()) - out_type = cr.get_output_kind(input_data_obj.keys()) + cr.validate_input(list(input_data_obj.keys())) + out_type = cr.get_output_kind(list(input_data_obj.keys())) assert "BOLD" in out_type @@ -240,8 +240,8 @@ def test_baseconfoundremover() -> None: cr = BaseConfoundRemover( strategy=strat4, spike=None, mask_img=simsk, t_r=0.75 ) - cr.validate_input(input_data_obj.keys()) - out_type = cr.get_output_kind(input_data_obj.keys()) + cr.validate_input(list(input_data_obj.keys())) + out_type = cr.get_output_kind(list(input_data_obj.keys())) assert "BOLD" in out_type @@ -273,8 +273,8 @@ def test_baseconfoundremover() -> None: cr = BaseConfoundRemover( strategy=strat5, spike=None, mask_img=simsk, t_r=0.75 ) - cr.validate_input(input_data_obj.keys()) - out_type = cr.get_output_kind(input_data_obj.keys()) + cr.validate_input(list(input_data_obj.keys())) + out_type = cr.get_output_kind(list(input_data_obj.keys())) assert "BOLD" in out_type @@ -320,8 +320,8 @@ def test_baseconfoundremover() -> None: cr = BaseConfoundRemover( strategy=strat6, spike=0.75, mask_img=simsk, t_r=0.75 ) - cr.validate_input(input_data_obj.keys()) - out_type = cr.get_output_kind(input_data_obj.keys()) + cr.validate_input(list(input_data_obj.keys())) + out_type = cr.get_output_kind(list(input_data_obj.keys())) assert "BOLD" in out_type diff --git a/junifer/stats.py b/junifer/stats.py index e0a30c0ce..aa55e7475 100644 --- a/junifer/stats.py +++ b/junifer/stats.py @@ -5,7 +5,7 @@ # License: AGPL from functools import partial -from typing import Callable, Dict +from typing import Callable, Dict, Any, Optional import numpy as np from scipy.stats import trim_mean @@ -14,7 +14,9 @@ from scipy.stats.mstats import winsorize from .utils import logger, raise_error -def get_aggfunc_by_name(name: str, func_params: Dict) -> Callable: +def get_aggfunc_by_name( + name: str, + func_params: Optional[Dict[str, Any]]) -> Callable: """Get an aggregation function by its name. Parameters @@ -38,11 +40,24 @@ def get_aggfunc_by_name(name: str, func_params: Dict) -> Callable: """ # check validity of names _valid_func_names = {"winsorized_mean", "mean", "std", "trim_mean"} - + if func_params is None: + func_params = {} # apply functions if name == "winsorized_mean": # check validity of func_params limits = func_params.get("limits") + if limits is None or not isinstance(limits, list): + raise_error( + "func_params must contain a list of limits for " + "winsorized_mean", + ValueError, + ) + if len(limits) != 2: + raise_error( + "func_params must contain a list of two limits for " + "winsorized_mean", + ValueError, + ) if all((lim >= 0.0 and lim <= 1) for lim in limits): logger.info(f"Limits for winsorized mean are set to {limits}.") else: @@ -69,7 +84,7 @@ def get_aggfunc_by_name(name: str, func_params: Dict) -> Callable: def winsorized_mean( - data: np.ndarray, axis: int = None, **win_params + data: np.ndarray, axis: Optional[int] = None, **win_params ) -> np.ndarray: """Compute a winsorized mean by chaining winsorization and mean. diff --git a/junifer/storage/base.py b/junifer/storage/base.py index 9130eeb02..46a2f0e40 100644 --- a/junifer/storage/base.py +++ b/junifer/storage/base.py @@ -182,7 +182,7 @@ class BaseFeatureStorage(ABC): data, meta: Dict, columns: Optional[Iterable[str]] = None, - rows_col_name: str = None, + rows_col_name: Optional[str] = None, ) -> None: """Store table. diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index f15903982..b08213323 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -20,7 +20,7 @@ from .utils import element_to_index, element_to_prefix, process_meta if TYPE_CHECKING: - from sqlalchemy import Engine + from sqlalchemy.engine import Engine @register_storage @@ -103,17 +103,18 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): meta = {} # Retrieve element key from metadata element = meta.get("element", None) - # Functionality check - if self.single_output is False and element is None: - raise_error( - msg="element must be specified when single_output is False." - ) # Prefixed elements prefix = "" if self.single_output is False: + if element is None: + raise_error( + msg="element must be specified when" + "single_output is False." + ) prefix = element_to_prefix(element) # Format URI for engine creation - uri = f"sqlite:///{self.uri.parent}/{prefix}{self.uri.name}" + uri = "sqlite:///" \ + f"{self.uri.parent}/{prefix}{self.uri.name}" # type: ignore return create_engine(uri, echo=False) def _save_upsert( @@ -211,7 +212,7 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): data, meta: Dict, columns: Optional[Iterable[str]] = None, - rows_col_name: str = None, + rows_col_name: Optional[str] = None, ) -> None: """Store 2D dataframe. @@ -234,7 +235,8 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): meta=meta, n_rows=n_rows, rows_col_name=rows_col_name ) # Prepare new dataframe - data_df = pd.DataFrame(data, columns=columns, index=idx) + data_df = pd.DataFrame( + data, columns=columns, index=idx) # type: ignore # Store dataframe self.store_df(df=data_df, meta=meta) @@ -405,7 +407,7 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): data, meta: Dict, col_names: Optional[Iterable[str]] = None, - rows_col_name: str = None, + rows_col_name: Optional[str] = None, ) -> None: """Implement 2D matrix storing. @@ -433,7 +435,7 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): data, meta: Dict, columns: Optional[Iterable[str]] = None, - rows_col_name: str = None, + rows_col_name: Optional[str] = None, ) -> None: """Implement table storing. @@ -528,13 +530,14 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): """ if self.single_output is True: raise_error(msg="collect() is not implemented for single output.") - logger.info(f"Collecting data from {self.uri.parent}/*{self.uri.name}") + logger.info("Collecting data from " + f"{self.uri.parent}/*{self.uri.name}") # type: ignore # Create new instance out_storage = SQLiteFeatureStorage( uri=self.uri, single_output=True, upsert="ignore" ) # Glob files - files = self.uri.parent.glob(f"*{self.uri.name}") + files = self.uri.parent.glob(f"*{self.uri.name}") # type: ignore for elem in tqdm(files, desc="file"): logger.debug(f"Reading from {str(elem.absolute())}") in_storage = SQLiteFeatureStorage(uri=elem, single_output=True) diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index aa33b9a8d..542b27632 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -4,6 +4,8 @@ # Synchon Mandal # License: AGPL +from typing import Union, List + from pathlib import Path import numpy as np @@ -57,7 +59,8 @@ df_ignore = pd.DataFrame( ).set_index(["element", "pk2"]) -def _read_sql(table_name: str, uri: str, index_col: str) -> pd.DataFrame: +def _read_sql(table_name: str, uri: str, + index_col: Union[str, List[str]]) -> pd.DataFrame: """Read database table into a pandas DataFrame. Parameters @@ -157,7 +160,8 @@ def test_upsert_replace(tmp_path: Path) -> None: table_name = storage.store_metadata(meta) # Read stored table c_df1 = _read_sql( - table_name=table_name, uri=uri, index_col=["element", "pk2"] + table_name=table_name, uri=uri.as_posix(), + index_col=["element", "pk2"] ) # Check if dataframes are equal assert_frame_equal(df1, c_df1) @@ -165,7 +169,8 @@ def test_upsert_replace(tmp_path: Path) -> None: storage._save_upsert(df=df2, name=table_name, if_exists="replace") # Read stored table c_df2 = _read_sql( - table_name=table_name, uri=uri, index_col=["element", "pk2"] + table_name=table_name, uri=uri.as_posix(), + index_col=["element", "pk2"] ) # Check if dataframes are equal assert_frame_equal(df2, c_df2) @@ -193,7 +198,9 @@ def test_upsert_ignore(tmp_path: Path) -> None: table_name = storage.store_metadata(meta=meta) # Read stored table c_df1 = _read_sql( - table_name=table_name, uri=uri, index_col=["element", "pk2"] + table_name=table_name, + uri=uri.as_posix(), + index_col=["element", "pk2"] ) # Check if dataframes are equal assert_frame_equal(df1, c_df1) @@ -201,7 +208,8 @@ def test_upsert_ignore(tmp_path: Path) -> None: with pytest.warns(RuntimeWarning, match="are already present"): storage.store_df(df2, meta) # Read stored table - c_dfignore = _read_sql(table_name, uri=uri, index_col=["element", "pk2"]) + c_dfignore = _read_sql( + table_name, uri=uri.as_posix(), index_col=["element", "pk2"]) # Check if dataframes are equal assert_frame_equal(c_dfignore, df_ignore) # Check for error @@ -228,14 +236,17 @@ def test_upsert_update(tmp_path: Path) -> None: table_name = storage.store_metadata(meta) # Read stored table c_df1 = _read_sql( - table_name=table_name, uri=uri, index_col=["element", "pk2"] + table_name=table_name, uri=uri.as_posix(), + index_col=["element", "pk2"] ) # Check if dataframes are equal assert_frame_equal(df1, c_df1) # Save to database storage.store_df(df2, meta) # Read stored table - c_dfupdate = _read_sql(table_name, uri=uri, index_col=["element", "pk2"]) + c_dfupdate = _read_sql( + table_name, uri=uri.as_posix(), + index_col=["element", "pk2"]) # Check if dataframes are equal assert_frame_equal(c_dfupdate, df_update) @@ -370,7 +381,8 @@ def test_store_table(tmp_path: Path) -> None: table_name = storage.store_metadata(meta) # Read stored table c_df = _read_sql( - table_name=table_name, uri=uri, index_col=["element", "scan"] + table_name=table_name, uri=uri.as_posix(), + index_col=["element", "scan"] ) # Check if dataframes are equal assert_frame_equal(df, c_df) @@ -388,7 +400,8 @@ def test_store_table(tmp_path: Path) -> None: ) # Read stored table c_df_new = _read_sql( - table_name=table_name, uri=uri, index_col=["element", "scan"] + table_name=table_name, uri=uri.as_posix(), + index_col=["element", "scan"] ) # Check if dataframes are equal assert_frame_equal(df_new, c_df_new) @@ -484,9 +497,9 @@ def test_store_multiple_output(tmp_path: Path): # Set index columns cols = ["subject", "session", "scan"] # Read stored tables - cdf1 = _read_sql(table_name, uri1, index_col=cols) - cdf2 = _read_sql(table_name, uri2, index_col=cols) - cdf3 = _read_sql(table_name, uri3, index_col=cols) + cdf1 = _read_sql(table_name, uri1.as_posix(), index_col=cols) + cdf2 = _read_sql(table_name, uri2.as_posix(), index_col=cols) + cdf3 = _read_sql(table_name, uri3.as_posix(), index_col=cols) # Check if dataframes are equal assert_frame_equal(df1, cdf1) assert_frame_equal(df2, cdf2) @@ -566,10 +579,10 @@ def test_collect(tmp_path: Path) -> None: # Store metadata table_name = storage.store_metadata(meta1) # Read stored tables - all_df = _read_sql(table_name, uri, index_col=cols) - cdf1 = _read_sql(table_name, uri1, index_col=cols) - cdf2 = _read_sql(table_name, uri2, index_col=cols) - cdf3 = _read_sql(table_name, uri3, index_col=cols) + all_df = _read_sql(table_name, uri.as_posix(), index_col=cols) + cdf1 = _read_sql(table_name, uri1.as_posix(), index_col=cols) + cdf2 = _read_sql(table_name, uri2.as_posix(), index_col=cols) + cdf3 = _read_sql(table_name, uri3.as_posix(), index_col=cols) # Operate on retrieved tables all_cdf = pd.concat([cdf1, cdf2, cdf3]) all_df.sort_index(level=cols, inplace=True) diff --git a/junifer/storage/tests/test_storage_base.py b/junifer/storage/tests/test_storage_base.py index 6a69d9503..c9af9db04 100644 --- a/junifer/storage/tests/test_storage_base.py +++ b/junifer/storage/tests/test_storage_base.py @@ -12,7 +12,7 @@ from junifer.storage.base import BaseFeatureStorage def test_BaseFeatureStorage_abstractness() -> None: """Test BaseFeatureStorage is abstract base class.""" with pytest.raises(TypeError, match=r"abstract"): - BaseFeatureStorage(uri="/tmp") + BaseFeatureStorage(uri="/tmp") # type: ignore def test_BaseFeatureStorage() -> None: @@ -74,7 +74,7 @@ def test_BaseFeatureStorage() -> None: st.store_table(None, None) with pytest.raises(NotImplementedError): - st.store_df(None, None) + st.store_df(None, None) # type: ignore with pytest.raises(NotImplementedError): st.store_timeseries(None, None) diff --git a/junifer/storage/tests/test_utils.py b/junifer/storage/tests/test_utils.py index 751410b85..a5b2b466d 100644 --- a/junifer/storage/tests/test_utils.py +++ b/junifer/storage/tests/test_utils.py @@ -19,7 +19,7 @@ def test_process_meta_invalid_metadata_type() -> None: """Test invalid metadata type check for metadata hash processing.""" meta = None with pytest.raises(ValueError, match=r"`meta` must be a dict"): - process_meta(meta) + process_meta(meta) # type: ignore # TODO: parameterize diff --git a/junifer/storage/utils.py b/junifer/storage/utils.py index b707e80ea..1d7585787 100644 --- a/junifer/storage/utils.py +++ b/junifer/storage/utils.py @@ -4,6 +4,8 @@ # Synchon Mandal # License: AGPL +from typing import Any + import hashlib import json from typing import Dict, Optional, Tuple, Union @@ -155,7 +157,7 @@ def element_to_index( # Check rows_col_name if rows_col_name is None: rows_col_name = "idx" - elem_idx = {k: [v] * n_rows for k, v in element.items()} + elem_idx: Dict[Any, Any] = {k: [v] * n_rows for k, v in element.items()} elem_idx[rows_col_name] = np.arange(n_rows) # Create index index = pd.MultiIndex.from_frame( diff --git a/junifer/utils/logging.py b/junifer/utils/logging.py index bb4e5627e..0dc6c0ccd 100644 --- a/junifer/utils/logging.py +++ b/junifer/utils/logging.py @@ -23,7 +23,7 @@ _logging_types = { } -class WrapStdOut: +class WrapStdOut(logging.StreamHandler): """ Dynamically wrap to sys.stdout. @@ -67,7 +67,7 @@ def _get_git_head(path: Path) -> str: ) try: stdout, _ = process.communicate(timeout=10) - proc_stdout = stdout.strip() + proc_stdout = stdout.strip().decode() except TimeoutExpired: process.kill() proc_stdout = "" @@ -95,7 +95,7 @@ def get_versions() -> Dict: if module_version is None: module_version = None elif "git" in module_version: - git_path = Path(module.__file__).resolve().parent + git_path = Path(module.__file__).resolve().parent # type: ignore head = _get_git_head(git_path) module_version += f"-HEAD:{head}" @@ -240,7 +240,7 @@ def configure_logging( mode = "w" if overwrite else "a" lh = logging.FileHandler(fname, mode=mode) else: - lh = logging.StreamHandler(WrapStdOut()) + lh = logging.StreamHandler(WrapStdOut()) # type: ignore # Set logging format if output_format is None: @@ -272,7 +272,9 @@ def raise_error(msg: str, klass: Type[Exception] = ValueError) -> NoReturn: raise klass(msg) -def warn_with_log(msg: str, category: Warning = RuntimeWarning) -> None: +def warn_with_log( + msg: str, + category: Optional[Type[Warning]] = RuntimeWarning) -> None: """Warn, but first log it. Parameters -- 2.52.0 From 63a993f6fcb7b408811c11f70a091d6f9bbd99a5 Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 8 Sep 2022 15:20:26 +0200 Subject: [PATCH 2/9] Fix doc --- docs/api.rst | 2 +- examples/norun_hcpfc_pearson.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 3aceb9ac7..a20d04331 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -13,5 +13,5 @@ Data Grabbers :members: .. autoclass:: junifer.datagrabber.datalad_base.DataladDataGrabber :members: -.. autoclass:: junifer.datagrabber.pattern_datalad_base.PatternDataladDataGrabber +.. autoclass:: junifer.datagrabber.pattern_datalad.PatternDataladDataGrabber :members: diff --git a/examples/norun_hcpfc_pearson.py b/examples/norun_hcpfc_pearson.py index 6875e23b9..e3c98e1ca 100644 --- a/examples/norun_hcpfc_pearson.py +++ b/examples/norun_hcpfc_pearson.py @@ -1,5 +1,4 @@ -"""extraction of FC from HCP data. - +""" HCP FC Extraction ====================== -- 2.52.0 From 888bb00012c80f4a6d89e78d2e868e62677435ae Mon Sep 17 00:00:00 2001 From: Fede Raimondo Date: Thu, 8 Sep 2022 15:21:49 +0200 Subject: [PATCH 3/9] Dman flake --- junifer/utils/logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/junifer/utils/logging.py b/junifer/utils/logging.py index 0dc6c0ccd..bcb79ef3e 100644 --- a/junifer/utils/logging.py +++ b/junifer/utils/logging.py @@ -273,7 +273,7 @@ def raise_error(msg: str, klass: Type[Exception] = ValueError) -> NoReturn: def warn_with_log( - msg: str, + msg: str, category: Optional[Type[Warning]] = RuntimeWarning) -> None: """Warn, but first log it. -- 2.52.0 From ece51af1b02a86f7dc6acc57fd2aa70e7cb4e23b Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Thu, 8 Sep 2022 16:48:05 +0200 Subject: [PATCH 4/9] docs: update README.md with shields and content --- README.md | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 64d11f49a..4c4773c31 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,18 @@ -# python-library-mockup -JUelich NeuroImaging FEature extractoR +# junifer - JUelich NeuroImaging FEature extractoR + +![PyPI](https://img.shields.io/pypi/v/junifer?style=flat-square) +![PyPI - Python Version](https://img.shields.io/pypi/pyversions/junifer?style=flat-square) +![PyPI - Wheel](https://img.shields.io/pypi/wheel/junifer?style=flat-square) +![GitHub](https://img.shields.io/github/license/juaml/junifer?style=flat-square) +[![codecov](https://codecov.io/gh/juaml/junifer/branch/main/graph/badge.svg?token=5H21JuZXMw)](https://codecov.io/gh/juaml/junifer) + +## About + +junifer is a data handling and feature extraction library targeted towards neuroimaging data specifically functional MRI data. + +It is curently being developed and maintained at the [Applied Machine Learning](https://www.fz-juelich.de/en/inm/inm-7/research-groups/applied-machine-learning-aml) group at [Forschungszentrum Juelich](https://www.fz-juelich.de/en), Germany. Although the library is designed for people working at [Institute of Neuroscience and Medicine - Brain and Behaviour (INM-7)](https://www.fz-juelich.de/en/inm/inm-7), it is designed to be as modular as possible thus enabling others to extend it easily. + +The documentation is available at [https://juaml.github.io/junifer](https://juaml.github.io/junifer/main/index.html). ## Repository Organization @@ -15,6 +28,39 @@ JUelich NeuroImaging FEature extractoR * `preprocess`: Preprocessing module. * `storage`: Storage module. * `utils`: Utilities module (e.g. logging) - - \ No newline at end of file + +## Installation + +Use `pip` to install from PyPI like so: + +``` +pip install junifer +``` + +## Citation + +If you use junifer in a scientific publication, we would appreciate if you cite our work. Currently, we do not have a publication, so feel free to use the project [URL](https://juaml.github.io/junifer). + +## Contribution + +Contributions are welcome and greatly appreciated. Please read the [guidelines](https://juaml.github.io/junifer/main/contributing.html) to get started. + +## License + +junifer is released under the AGPL v3 license: + +julearn, FZJuelich AML neuroimaging feature extraction library. +Copyright (C) 2022, authors of junifer. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . -- 2.52.0 From c183095f3151a637135dce2e7dcd8c2220affa5b Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Fri, 9 Sep 2022 15:10:22 +0200 Subject: [PATCH 5/9] docs: add about for junifer in index.rst --- docs/index.rst | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 3deeb2c34..1c44c3944 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -1,8 +1,17 @@ .. include:: links.inc -Welcome to the documentation! -============================= +Welcome to junifer's documentation! +=================================== +junifer (JUelich NeuroImaging FEature extractoR) is a data handling and feature +extraction library targeted towards neuroimaging data specifically functional +MRI data. + +It is curently being developed and maintained at the Applied Machine Learning +(`AML`_) group at Forschungszentrum Juelich, Germany. Although the library is +designed for people working at Institute of Neuroscience and Medicine - Brain +and Behaviour (`INM-7`_), it is designed to be as modular as possible thus +enabling others to extend it easily. .. toctree:: :maxdepth: 2 -- 2.52.0 From 562189b1a7de0bcc48e2002095129cf8d3b25489 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Fri, 9 Sep 2022 16:02:25 +0200 Subject: [PATCH 6/9] docs: update installation.rst --- docs/installation.rst | 86 ++++++++++++------------------------------- 1 file changed, 24 insertions(+), 62 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index 99ddee684..d257b6042 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -1,91 +1,53 @@ .. include:: links.inc -Installing -========== +Installing junifer +================== Requirements ^^^^^^^^^^^^ -junifer requires the following packages: +junifer is compatible with `Python`_ >= 3.8 and requires the following packages: -Running the examples requires: +* click>=8.1.3,<8.2 +* numpy>=1.22,<1.23 +* datalad>=0.15.4,<0.18 +* pandas>=1.4.0,<1.5 +* nibabel>=3.2.0,<4.1 +* nilearn>=0.9.0,<1.0 +* sqlalchemy>=1.4.27,<= 1.5.0 +* pyyaml>=5.1.2,<7.0 -Depending on the installation method, this packages might be installed -automatically. +Depending on the installation method, these packages might be installed automatically. -Installing -^^^^^^^^^^ -There are different ways to install junifer: +Installation +^^^^^^^^^^^^ +Depending on your use-case, junifer can be installed differently. * Install the :ref:`install_latest_release`. This is the most suitable approach - for most end users. -* Install the :ref:`install_latest_development`. This version will have the - latest features. However, it is still under development and not yet - officially released. Some features might still change before the next stable - release. -* Install from :ref:`install_development_git`. This is mostly suitable for - developers that want to have the latest version and yet edit the code. + for end users. +* Install from :ref:`install_development_git`. This is mostly suitable approach + for developers. -Either way, we strongly recommend using virtual environments: - -* `venv`_ -* `conda env`_ +Either way, we strongly recommend using `virtual environments `_. .. _install_latest_release: -Latest release +Stable release -------------- -We have packaged junifer and published it in PyPi, so you can just install it -with `pip`. +Use `pip` to install julearn from PyPI, like so: .. code-block:: bash pip install -U junifer -.. _install_latest_development: - -Latest Development Version --------------------------- -First, make sure that you have all the dependencies installed: - -Then, install junifer from TestPypi - -.. code-block:: bash - - pip install -U junifer --pre - - .. _install_development_git: -Local git repository (for developers) -------------------------------------- -First, make sure that you have all the dependencies installed: +Local Git repository +-------------------- -Then, clone `junifer Github`_ repository in a folder of your choice: - -.. code-block:: bash - - git clone https://github.com/juaml/junifer.git - -Install development mode requirements: - -.. code-block:: bash - - cd junifer - pip install -r dev-requirements.txt - -Finally, install in development mode: - -.. code-block:: bash - - python setup.py develop - -.. note:: Every time that you run ``setup.py develop``, the version is going to - be automatically set based on the git history. Nevertheless, this change - should not be committed (changes to ``_version.py``). Running ``git stash`` - at this point will forget the local changes to ``_version.py``. +Follow the `detailed contribution guidelines `_. -- 2.52.0 From 5ad92fa2f527b6e1c027faf69f3b91992db5e939 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Fri, 9 Sep 2022 16:03:03 +0200 Subject: [PATCH 7/9] docs: add contribution.rst --- docs/contribution.rst | 187 ++++++++++++++++++++++++++++++++++++++++++ docs/index.rst | 1 + 2 files changed, 188 insertions(+) create mode 100644 docs/contribution.rst diff --git a/docs/contribution.rst b/docs/contribution.rst new file mode 100644 index 000000000..e68c8123d --- /dev/null +++ b/docs/contribution.rst @@ -0,0 +1,187 @@ +.. include:: links.inc + +Contributing to junifer +======================= + + +Setting up the local development environment +-------------------------------------------- + +1. Fork the https://github.com/juaml/junifer repository on GitHub. If you + have never done this before, `follow the official guide + `_. +2. Clone your fork locally as described in the same guide. +3. Install your local copy into a Python virtual environment. You can `read + this guide to learn more + `_ about them + and how to create one. + + .. code-block:: console + + pip install -e ".[dev]" + +4. Create a branch for local development using the ``dev`` branch as a + starting point. Use ``fix``, ``refactor``, or ``feat`` as a prefix. + + .. code-block:: console + + git checkout dev + git checkout -b / + + Now you can make your changes locally. + +5. When making changes locally, it is helpful to ``git commit`` your work + regularly. On one hand to save your work and on the other hand, the smaller + the steps, the easier it is to review your work later. Please use `semantic + commit messages + `_. + + .. code-block:: console + + git add . + git commit -m ": " + +6. When you're done making changes, check that your changes pass our test suite. + This is all included with ``tox``. + + .. code-block:: console + + tox + + You can also run all ``tox`` tests in parallel. As of ``tox 3.7``, you can run:: + + .. code-block:: console + + tox --parallel + + +7. Push your branch to GitHub. + + .. code-block:: console + + git push origin / + +8. Open the link displayed in the message when pushing your new branch in order + to submit a pull request. Please follow the template presented to you in the + web interface to complete your pull request. + + +GitHub Pull Request guidelines +------------------------------ + +Before you submit a pull request, check that it meets these guidelines: + +1. The pull request should include tests in the respective ``tests`` directory. + Except in rare circumstances, code coverage must not decrease (as reported + by codecov which runs automatically when you submit your pull request). +2. If the pull request adds functionality, the docs should be + updated. Consider creating a Python file that demonstrates the usage in + ``examples`` directory. +3. The pull request should also include a short one-liner of your contribution + in `docs/changes/latest.inc`. If it's your first contribution, also add + yourself to `docs/changes/contributors.inc`. +3. The pull request will be tested for several different Python versions. +4. Someone from the core team will review your work and guide you to a successful + contribution. + + +Running unit tests +------------------ + +junifer uses `pytest `_ for its +unit-tests and new features should in general always come with new +tests that make sure that the code runs as intended. + +To run all tests:: + +.. code-block:: console + + tox -e test + + +Adding and building documentation +--------------------------------- + +Building the documentation requires some extra packages and can be installed by:: + +.. code-block:: console + + pip install -e ".[docs]" + +To build the docs:: + +.. code-block:: bash + + cd docs + make html + +To view the documentation, open `docs/_build/html/index.html`. + +In case you remove some files or change their filenames, you can run into +errors when using ``make html``. In this situation you can use ``make clean`` +to clean up the already build files and then re-run ``make html``. + + +Writing Examples +---------------- + +Examples are run and displayed in HTML format using `sphinx gallery`_. To add an +example, just create a ``.py`` file that starts either with ``plot_`` or ``run_``, +dependending on whether the example generates a figure or not. + +The first lines of the example should be a python block comment with a title, +a description of the example an the following include directive to be able to +use the links. + +The format used for text is reST. Check the `sphinx reST reference`_ for more +details. + +Example of the first lines: + + +.. code-block:: python + + """ + Simple Binary Classification + ============================ + + This example uses the 'iris' dataset and performs a simple binary + classification using a Support Vector Machine classifier. + + .. include:: ../../links.inc + """ + + +The rest of the script will be executed as normal Python code. In order to +render the output and embed formatted text within the code, you need to add +a 79 ``#`` (a full line) at the point in which you want to render and add text. +Each line of text shall be preceded with ``#``. The code that is not +commented will be executed. + +The following example will create 3 texts and render the output between the +texts. + +.. code-block:: python + + ############################################################################### + # Imports needed for the example + from seaborn import load_dataset + from julearn import run_cross_validation + from julearn.utils import configure_logging + + ############################################################################### + df_iris = load_dataset('iris') + + ############################################################################### + # The dataset has three kind of species. We will keep two to perform a binary + # classification. + df_iris = df_iris[df_iris['species'].isin(['versicolor', 'virginica'])] + + +Finally, when the example is done, you can run as a normal Python script. +To generate the HTML, just build the docs: + +.. code-block:: bash + + cd docs + make html diff --git a/docs/index.rst b/docs/index.rst index 1c44c3944..7e0aa03ef 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -21,6 +21,7 @@ enabling others to extend it easily. data api auto_examples/index.rst + contribution maintaining whats_new -- 2.52.0 From c3c503f34f1fe1f9c830f25bfdeb82848787a12c Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Fri, 9 Sep 2022 16:14:30 +0200 Subject: [PATCH 8/9] docs: fix numbering in contribution.rst --- docs/contribution.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contribution.rst b/docs/contribution.rst index e68c8123d..1e77d21c0 100644 --- a/docs/contribution.rst +++ b/docs/contribution.rst @@ -80,8 +80,8 @@ Before you submit a pull request, check that it meets these guidelines: 3. The pull request should also include a short one-liner of your contribution in `docs/changes/latest.inc`. If it's your first contribution, also add yourself to `docs/changes/contributors.inc`. -3. The pull request will be tested for several different Python versions. -4. Someone from the core team will review your work and guide you to a successful +4. The pull request will be tested against several Python versions. +5. Someone from the core team will review your work and guide you to a successful contribution. -- 2.52.0 From b47dd8a473af4fd4c364cc9fc9f8b552477d5f09 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Fri, 9 Sep 2022 16:14:48 +0200 Subject: [PATCH 9/9] docs: fix links.inc --- docs/links.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/links.inc b/docs/links.inc index 27414754b..444454691 100644 --- a/docs/links.inc +++ b/docs/links.inc @@ -30,4 +30,4 @@ .. _`setuptools_scm`: https://github.com/pypa/setuptools_scm/ .. _`sphinx gallery`: https://sphinx-gallery.github.io/stable/index.html -.. _`sphinx RST reference`: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#inline-markup \ No newline at end of file +.. _`sphinx reST reference`: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#inline-markup -- 2.52.0