Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add deprecation warnings to BidsDataset properties slated for change #251

Merged
merged 5 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 54 additions & 15 deletions snakebids/core/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

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

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
Expand Down Expand Up @@ -200,6 +202,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[\\<component_name\\>].path <BidsComponent.path>`
""",
deprecated_in="0.8.0",
admonition="warning",
)
def path(self):
"""Dict mapping :class:`BidsComponents <snakebids.BidsComponent>` names to \
their ``paths``.
Expand All @@ -211,6 +222,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[\\<component_name\\>].zip_lists <BidsComponent.zip_lists>`
""",
deprecated_in="0.8.0",
admonition="warning",
)
def zip_lists(self):
"""Dict mapping :class:`BidsComponents <snakebids.BidsComponent>` names to \
their ``zip_lists``
Expand All @@ -222,6 +243,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[\\<component_name\\>].entities <BidsComponent.entities>`
""",
deprecated_in="0.8.0",
admonition="warning",
)
def entities(self):
"""Dict mapping :class:`BidsComponents <snakebids.BidsComponent>` names to \
to their :attr:`entities <snakebids.BidsComponent.entities>`
Expand All @@ -233,6 +264,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[\\<component_name\\>].wildcards <BidsComponent.wildcards>`
""",
deprecated_in="0.8.0",
admonition="warning",
)
def wildcards(self):
"""Dict mapping :class:`BidsComponents <snakebids.BidsComponent>` names to \
their :attr:`wildcards <snakebids.BidsComponent.wildcards>`
Expand All @@ -249,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()
)
}
]
Expand All @@ -262,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()
)
}
]
Expand Down Expand Up @@ -294,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]):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ 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:
bids(
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}'
Expand All @@ -56,6 +56,6 @@ rule all:
allow_missing=True,
),
zip,
**inputs.zip_lists['bold']
**inputs['bold'].zip_lists
)
default_target: True
13 changes: 8 additions & 5 deletions snakebids/tests/test_datasets.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import copy
import itertools as it
import warnings
from typing import Any, Dict, List

import more_itertools as itx
Expand Down Expand Up @@ -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.entities
assert dataset.input_wildcards == dataset.wildcards


class TestBidsComponentValidation:
Expand Down
25 changes: 11 additions & 14 deletions snakebids/tests/test_generate_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()))

Expand Down Expand Up @@ -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}"}
Expand All @@ -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}"}

Expand Down Expand Up @@ -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}"}

Expand Down Expand Up @@ -647,7 +644,7 @@ def test_t1w():
{
"t1": BidsComponent(
"t1",
result.input_path["t1"],
result["t1"].path,
{"acq": ["mprage", "mprage"], "subject": ["001", "002"]},
)
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
18 changes: 16 additions & 2 deletions snakebids/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
-------
Expand All @@ -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
Expand Down
27 changes: 27 additions & 0 deletions typings/deprecation.pyi
Original file line number Diff line number Diff line change
@@ -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: ...