From fe2bd79610bf37316a4af8a1f4ae341f8c6cafe4 Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Sat, 18 Feb 2023 16:04:00 -0500 Subject: [PATCH 1/5] Add deprecation library and type stub Currently using a custom fork of the library to get allow custom deprecation admonitions in the docs --- poetry.lock | 21 ++++++++++++++++++++- pyproject.toml | 1 + typings/deprecation.pyi | 27 +++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 typings/deprecation.pyi diff --git a/poetry.lock b/poetry.lock index 607bf4b2..493af1c8 100644 --- a/poetry.lock +++ b/poetry.lock @@ -233,6 +233,24 @@ category = "main" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" +[[package]] +name = "deprecation" +version = "2.1.0" +description = "A library to handle automated deprecations" +category = "main" +optional = false +python-versions = "*" +develop = false + +[package.dependencies] +packaging = "*" + +[package.source] +type = "git" +url = "https://github.com/pvandyken/deprecation.git" +reference = "updates" +resolved_reference = "8309c44112f349df9c38cbbfea6aed73f99ff66c" + [[package]] name = "dill" version = "0.3.6" @@ -1701,7 +1719,7 @@ testing = ["flake8 (<5)", "func-timeout", "jaraco.functools", "jaraco.itertools" [metadata] lock-version = "1.1" python-versions = ">=3.7,<3.12" -content-hash = "51d46ab2413d0a1cdb6b3581e8f1e7d67a6dacd302bfc2510f334570b194df59" +content-hash = "ce7ac5231dbff1af17bd4bf8f0679443ce94377a84b2e0e7b8e4e4c491d47ccd" [metadata.files] appdirs = [ @@ -1918,6 +1936,7 @@ datrie = [ {file = "datrie-0.8.2-pp373-pypy36_pp73-win32.whl", hash = "sha256:89ff3d41df4f899387aa07b4b066f5da36e3a10b67b8aeae631c950502ff4503"}, {file = "datrie-0.8.2.tar.gz", hash = "sha256:525b08f638d5cf6115df6ccd818e5a01298cd230b2dac91c8ff2e6499d18765d"}, ] +deprecation = [] dill = [ {file = "dill-0.3.6-py3-none-any.whl", hash = "sha256:a07ffd2351b8c678dfc4a856a3005f8067aea51d6ba6c700796a4d9e280f39f0"}, {file = "dill-0.3.6.tar.gz", hash = "sha256:e5db55f3687856d8fbdab002ed78544e1c4559a130302693d839dfe8f93f2373"}, diff --git a/pyproject.toml b/pyproject.toml index 0c5a13ce..81598a49 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ attrs = ">=21.2.0,<23" boutiques = "^0.5.25" more-itertools = ">=8,<10" cached-property = "^1.5.2" +deprecation = { git = "https://github.com/pvandyken/deprecation.git", rev="8309c44" } # Below are non-direct dependencies (i.e. dependencies of other depenencies) # specified to ensure a version with a pre-built wheel is installed depending diff --git a/typings/deprecation.pyi b/typings/deprecation.pyi new file mode 100644 index 00000000..e7fefe47 --- /dev/null +++ b/typings/deprecation.pyi @@ -0,0 +1,27 @@ +from datetime import date +from typing import Callable, Generic, ParamSpec, TypeVar + +P = ParamSpec("P") +T = TypeVar("T", bound=Callable) + +class DeprecatedWarning(DeprecationWarning, Generic[T]): + def __init__( + self, + function: T, + deprecated_in: str, + removed_in: str | date | None, + details: str = ..., + ): ... + def __str__(self) -> str: ... + +class UnsupportedWarning(DeprecatedWarning): + def __str__(self) -> str: ... + +def deprecated( + deprecated_in: str | None = ..., + removed_in: str | date | None = ..., + current_version: str | None = ..., + details: str | None = ..., + admonition: str | None = ..., +) -> Callable[[T], T]: ... +def fail_if_not_removed(method: T) -> T: ... From 4f3c429cbe5e24b59b09e1adae9dd2777f81f302 Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Sat, 18 Feb 2023 16:05:58 -0500 Subject: [PATCH 2/5] Add deprecation warnings to BidsDataset props Deprecate the Component getter properties (that currently allow dataset->attr->component) to allow for future modification of those properties --- snakebids/core/datasets.py | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/snakebids/core/datasets.py b/snakebids/core/datasets.py index e6a9d2ff..ab27b1ec 100644 --- a/snakebids/core/datasets.py +++ b/snakebids/core/datasets.py @@ -9,6 +9,7 @@ import attr import more_itertools as itx from cached_property import cached_property +from deprecation import deprecated from typing_extensions import TypedDict import snakebids.utils.sb_itertools as sb_it @@ -200,6 +201,15 @@ def __setitem__(self, _: Any, __: Any): ) @cached_property + @deprecated( + details=""" + The behaviour of path will change in an upcoming release, where it will refer + instead to the root path of the dataset. Please access component paths using + :attr:`Dataset[\\].path ` + """, + deprecated_in="0.8.0", + admonition="warning", + ) def path(self): """Dict mapping :class:`BidsComponents ` names to \ their ``paths``. @@ -211,6 +221,16 @@ def input_path(self): return self.path @cached_property + @deprecated( + details=""" + The behaviour of zip_lists will change in an upcoming release, where it will + refer instead to the consensus of entity groups across all components in the + dataset. Please access component zip_lists using + :attr:`Dataset[\\].zip_lists ` + """, + deprecated_in="0.8.0", + admonition="warning", + ) def zip_lists(self): """Dict mapping :class:`BidsComponents ` names to \ their ``zip_lists`` @@ -222,6 +242,16 @@ def input_zip_lists(self): return self.zip_lists @cached_property + @deprecated( + details=""" + The behaviour of entities will change in the 1.0 release, where it will refer + instead to the union of all entity-values across all components in the dataset. + Please access component entity lists using + :attr:`Dataset[\\].entities ` + """, + deprecated_in="0.8.0", + admonition="warning", + ) def entities(self): """Dict mapping :class:`BidsComponents ` names to \ to their :attr:`entities ` @@ -233,6 +263,16 @@ def input_lists(self): return self.entities @cached_property + @deprecated( + details=""" + The behaviour of wildcards will change in an upcoming release, where it will + refer instead to the union of all entity-wildcard mappings across all components + in the dataset. Please access component wildcards using + :attr:`Dataset[\\].wildcards ` + """, + deprecated_in="0.8.0", + admonition="warning", + ) def wildcards(self): """Dict mapping :class:`BidsComponents ` names to \ their :attr:`wildcards ` From e730ac6fb26ab854c98bde86481d89b6db62a85a Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Sat, 18 Feb 2023 16:24:43 -0500 Subject: [PATCH 3/5] Change property_alias to copy just the summary The entire docstring can still be copied using the copy_extended_docstring property --- snakebids/utils/utils.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/snakebids/utils/utils.py b/snakebids/utils/utils.py index ca061ea5..3b414854 100644 --- a/snakebids/utils/utils.py +++ b/snakebids/utils/utils.py @@ -232,7 +232,12 @@ class _Documented(Protocol): __doc__: str -def property_alias(prop: _Documented, label: str | None = None, ref: str | None = None): +def property_alias( + prop: _Documented, + label: str | None = None, + ref: str | None = None, + copy_extended_docstring: bool = False, +): """Set property as an alias for another property Copies the docstring from the aliased property to the alias @@ -241,6 +246,12 @@ def property_alias(prop: _Documented, label: str | None = None, ref: str | None ---------- prop : property Property to alias + label + Text to use in link to aliased property + ref + Name of the property to alias + copy_extended_docstring + If True, copies over the entire docstring, in addition to the summary line Returns ------- @@ -254,7 +265,10 @@ def inner(__func: Callable[[Any], T]) -> "UserProperty[T]": else: link = f":attr:`{ref}`" if ref else None labeltxt = f"Alias of {link}\n\n" if link else "" - alias.__doc__ = f"{labeltxt}{prop.__doc__}" + if copy_extended_docstring: + alias.__doc__ = f"{labeltxt}{prop.__doc__}" + else: + alias.__doc__ = f"{labeltxt}{prop.__doc__.splitlines()[0]}" return alias return inner From ab34462562b2417a0a1b6ed946ba1ef6d0675924 Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Sat, 18 Feb 2023 17:40:02 -0500 Subject: [PATCH 4/5] Eliminate deprecated code Remove existing references to the deprecated BidsDataset properties. Some functions, including the tests of these properties, call them by necessity, so DeprecationWarnings are ignored at these points --- snakebids/core/datasets.py | 29 +++++++++---------- .../workflow/Snakefile | 8 ++--- snakebids/tests/test_datasets.py | 13 +++++---- snakebids/tests/test_generate_inputs.py | 25 +++++++--------- 4 files changed, 37 insertions(+), 38 deletions(-) diff --git a/snakebids/core/datasets.py b/snakebids/core/datasets.py index ab27b1ec..e24915fc 100644 --- a/snakebids/core/datasets.py +++ b/snakebids/core/datasets.py @@ -2,6 +2,7 @@ import itertools as it import operator as op +import warnings from collections import UserDict from string import Formatter from typing import TYPE_CHECKING, Any, Iterable, Optional, Union, cast @@ -289,9 +290,7 @@ def subjects(self): return [ *{ *it.chain.from_iterable( - input_list["subject"] - for input_list in self.entities.values() - if "subject" in input_list + component.entities.get("subject", []) for component in self.values() ) } ] @@ -302,9 +301,7 @@ def sessions(self): return [ *{ *it.chain.from_iterable( - input_list["session"] - for input_list in self.entities.values() - if "session" in input_list + component.entities.get("session", []) for component in self.values() ) } ] @@ -334,15 +331,17 @@ def as_dict(self): ------- BidsDatasetDict """ - return BidsDatasetDict( - input_path=self.input_path, - input_lists=self.entities, - input_wildcards=self.input_wildcards, - input_zip_lists=self.input_zip_lists, - subjects=self.subjects, - sessions=self.sessions, - subj_wildcards=self.subj_wildcards, - ) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + return BidsDatasetDict( + input_path=self.input_path, + input_lists=self.entities, + input_wildcards=self.input_wildcards, + input_zip_lists=self.input_zip_lists, + subjects=self.subjects, + sessions=self.sessions, + subj_wildcards=self.subj_wildcards, + ) @classmethod def from_iterable(cls, iterable: Iterable[BidsComponent]): diff --git a/snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile b/snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile index 9eb7661b..a09a583f 100644 --- a/snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile +++ b/snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile @@ -26,14 +26,14 @@ wildcard_constraints: **snakebids.get_wildcard_constraints(config['pybids_input rule smooth: - input: inputs.path['bold'] + input: inputs['bold'].path output: bids( root=config['root'], datatype='func', desc='smooth{fwhm}mm', suffix='bold.nii.gz', - **inputs.wildcards['bold'] + **inputs['bold'].wildcards ) container: config['singularity']['fsl'] log: @@ -41,7 +41,7 @@ rule smooth: root='logs', suffix='smooth.log', fwhm='{fwhm}', - **inputs.wildcards['bold'] + **inputs['bold'].wildcards ) params: sigma = lambda wildcards: f'{float(wildcards.fwhm)/2.355:0.2f}' shell: 'fslmaths {input} -s {params.sigma} {output}' @@ -56,6 +56,6 @@ rule all: allow_missing=True, ), zip, - **inputs.zip_lists['bold'] + **inputs['bold'].zip_lists ) default_target: True diff --git a/snakebids/tests/test_datasets.py b/snakebids/tests/test_datasets.py index b6b837e9..9fe4ae47 100644 --- a/snakebids/tests/test_datasets.py +++ b/snakebids/tests/test_datasets.py @@ -1,5 +1,6 @@ import copy import itertools as it +import warnings from typing import Any, Dict, List import more_itertools as itx @@ -31,11 +32,13 @@ def test_bids_component_aliases_are_correctly_set(self, component: BidsComponent @given(sb_st.bids_components()) def test_bids_dataset_aliases_are_correctly_set(self, component: BidsComponent): - dataset = BidsDataset.from_iterable([component]) - assert dataset.input_path == dataset.path - assert dataset.input_zip_lists == dataset.zip_lists - assert dataset.input_lists == dataset.input_lists - assert dataset.input_wildcards == dataset.wildcards + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + dataset = BidsDataset.from_iterable([component]) + assert dataset.input_path == dataset.path + assert dataset.input_zip_lists == dataset.zip_lists + assert dataset.input_lists == dataset.input_lists + assert dataset.input_wildcards == dataset.wildcards class TestBidsComponentValidation: diff --git a/snakebids/tests/test_generate_inputs.py b/snakebids/tests/test_generate_inputs.py index 1d80f5db..e741186c 100644 --- a/snakebids/tests/test_generate_inputs.py +++ b/snakebids/tests/test_generate_inputs.py @@ -58,7 +58,6 @@ def tmpdir(self, fakefs_tmpdir: Path): def disambiguate_components(self, dataset: BidsDataset): assert len(dataset) == 2 - dataset.zip_lists comp1, comp2 = dataset.values() return sorted([comp1, comp2], key=lambda comp: len(comp.entities.keys())) @@ -271,9 +270,7 @@ def test_missing_filters(self, tmpdir: Path): pybids_config=str(Path(__file__).parent / "data" / "custom_config.json"), use_bids_inputs=True, ) - template = BidsDataset( - {"t1": BidsComponent("t1", config.input_path["t1"], zip_list)} - ) + template = BidsDataset({"t1": BidsComponent("t1", config["t1"].path, zip_list)}) # Order of the subjects is not deterministic assert template == config assert config.subj_wildcards == {"subject": "{subject}"} @@ -297,7 +294,7 @@ def test_missing_wildcards(self, tmpdir: Path): pybids_config=str(Path(__file__).parent / "data" / "custom_config.json"), use_bids_inputs=True, ) - template = BidsDataset({"t1": BidsComponent("t1", config.input_path["t1"], {})}) + template = BidsDataset({"t1": BidsComponent("t1", config["t1"].path, {})}) assert template == config assert config.subj_wildcards == {"subject": "{subject}"} @@ -573,7 +570,7 @@ def test_custom_pybids_config(tmpdir: Path): } ) assert template == result - assert result.input_wildcards == {"t1": {"foo": "{foo}", "subject": "{subject}"}} + assert result["t1"].wildcards == {"foo": "{foo}", "subject": "{subject}"} # Order of the subjects is not deterministic assert result.subj_wildcards == {"subject": "{subject}"} @@ -647,7 +644,7 @@ def test_t1w(): { "t1": BidsComponent( "t1", - result.input_path["t1"], + result["t1"].path, {"acq": ["mprage", "mprage"], "subject": ["001", "002"]}, ) } @@ -678,14 +675,16 @@ def test_t1w(): participant_label="001", use_bids_inputs=True, ) - assert result.entities == { - "scan": {"acq": ["mprage"], "subject": ["001"], "suffix": ["T1w"]} + assert result["scan"].entities == { + "acq": ["mprage"], + "subject": ["001"], + "suffix": ["T1w"], } template = BidsDataset( { "scan": BidsComponent( "scan", - result.input_path["scan"], + result["scan"].path, { "acq": [ "mprage", @@ -751,15 +750,13 @@ def test_t1w(): { "t1": BidsComponent( "t1", - result.input_path["t1"], + result["t1"].path, { "acq": ["mprage", "mprage"], "subject": ["001", "002"], }, ), - "t2": BidsComponent( - "t2", result.input_path["t2"], {"subject": ["002"]} - ), + "t2": BidsComponent("t2", result["t2"].path, {"subject": ["002"]}), } ) # Order of the subjects is not deterministic From 402d3a48e57abe3a4031e62ca873f043a57a9364 Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Tue, 21 Feb 2023 11:10:43 -0500 Subject: [PATCH 5/5] Fix alias tests The test was checking if dataset.input_lists was equal to itself, rather than to dataset.entities --- snakebids/tests/test_datasets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snakebids/tests/test_datasets.py b/snakebids/tests/test_datasets.py index 9fe4ae47..a8b4ea73 100644 --- a/snakebids/tests/test_datasets.py +++ b/snakebids/tests/test_datasets.py @@ -37,7 +37,7 @@ def test_bids_dataset_aliases_are_correctly_set(self, component: BidsComponent): dataset = BidsDataset.from_iterable([component]) assert dataset.input_path == dataset.path assert dataset.input_zip_lists == dataset.zip_lists - assert dataset.input_lists == dataset.input_lists + assert dataset.input_lists == dataset.entities assert dataset.input_wildcards == dataset.wildcards