Skip to content

Commit

Permalink
Add deprecation and dangerous bids usage warnings
Browse files Browse the repository at this point in the history
Warn if `use_subject_dir` or `use_session_dir` is specified, these will
be eliminated in 1.0

Warn if custom entitites are used in a spec when no spec is explicitly
identified
  • Loading branch information
pvandyken committed Dec 11, 2023
1 parent 45bfbc6 commit eab98c7
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 18 deletions.
2 changes: 2 additions & 0 deletions snakebids/io/console.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"""
from __future__ import annotations

import functools as ft
from shutil import get_terminal_size


Expand Down Expand Up @@ -85,6 +86,7 @@ def get_console_size() -> tuple[int | None, int | None]:
# Detect our environment


@ft.lru_cache
def in_interactive_session() -> bool:
"""
Check if we're running in an interactive shell.
Expand Down
27 changes: 12 additions & 15 deletions snakebids/paths/_config.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
from __future__ import annotations

from typing import Literal, TypedDict, cast
from typing import TYPE_CHECKING, Literal, TypedDict, cast

from typing_extensions import TypeAlias

from snakebids.paths import specs
from snakebids.paths._factory import BidsFunction, bids_factory
from snakebids.paths._utils import BidsPathSpec

if TYPE_CHECKING:
from snakebids.paths._utils import BidsPathSpec

# <AUTOUPDATE>
# The code between these tags is automatically generated. Do not
Expand All @@ -25,15 +27,6 @@
class _Config(TypedDict):
active_spec: BidsPathSpec
bids_func: BidsFunction
explicit_spec: bool


_latest_spec = specs.latest()
_config = _Config(
active_spec=_latest_spec,
bids_func=bids_factory(_latest_spec, _v0_0_0=True),
explicit_spec=False,
)


def set_bids_spec(spec: BidsPathSpec | VALID_SPECS):
Expand All @@ -49,7 +42,6 @@ def set_bids_spec(spec: BidsPathSpec | VALID_SPECS):
spec = cast("BidsPathSpec", getattr(specs, spec)())
_config["active_spec"] = spec
_config["bids_func"] = bids_factory(spec)
_config["explicit_spec"] = True


def get_bids_spec() -> BidsPathSpec:
Expand All @@ -62,6 +54,11 @@ def get_bids_func() -> BidsFunction:
return _config["bids_func"]


def is_explicit_spec() -> bool:
"""Return True if BIDS spec was explicitly set"""
return _config["explicit_spec"]
def reset_bids_spec():
spec = specs.latest()
_config["active_spec"] = spec
_config["bids_func"] = bids_factory(spec, _implicit=True)


_config: _Config = {} # type: ignore
reset_bids_spec()
32 changes: 31 additions & 1 deletion snakebids/paths/_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
import itertools as it
import os
import sys
import warnings
from pathlib import Path
from typing import Protocol

import more_itertools as itx

from snakebids.io.console import in_interactive_session
from snakebids.paths import specs
from snakebids.paths._utils import BidsPathSpec, find_entity


Expand Down Expand Up @@ -51,7 +54,9 @@ def parse_entities(entities: dict[str, str | bool]) -> dict[str, str]:
return parse_entities


def bids_factory(spec: BidsPathSpec, *, _v0_0_0: bool = False) -> BidsFunction:
def bids_factory(
spec: BidsPathSpec, *, _v0_0_0: bool = False, _implicit: bool = False
) -> BidsFunction:
"""Factory generating bids functions according to the supplied spec
Parameters
Expand All @@ -61,6 +66,9 @@ def bids_factory(spec: BidsPathSpec, *, _v0_0_0: bool = False) -> BidsFunction:
_v0_0_0
Provides backward compatibility for the bids_v0_0_0 signature. Should not
otherwise be used
_implicit
Flag used internally to mark the default generated bids function. The
resulting builder will warn when custom entities are used
"""

order: list[str] = []
Expand Down Expand Up @@ -130,6 +138,16 @@ def bids(
include_session_dir ^ session_dir_default
or include_subject_dir ^ subject_dir_default
):
wrn_msg = (
"include_session_dir and include_subject_dir are deprecated and "
"will be removed in a future release. Builder functions without "
"directories can be created using the bids_factory and spec "
"functions:\n"
" from snakebids.paths import bids_factory, specs\n"
" bids = bids_factory(specs.v0_0_0(subject_dir=False, "
"session_dir=False))"
)
warnings.warn(wrn_msg)
return bids_factory(v0_0_0(include_subject_dir, include_session_dir))(
root,
datatype=datatype,
Expand Down Expand Up @@ -177,6 +195,18 @@ def bids(
for key, value in parsed.items():
custom_parts.append(f"{key}-{value}")

if custom_parts and _implicit and not in_interactive_session():
wrn_msg = (
f"The segment '{custom_parts}' has custom entities not part of the "
"current BIDS spec, but a spec has not been explicitly declared. This "
"could break when snakebids is upgraded, as specs can be updated "
"without warning, and these entities may be included in future specs. "
"Please declare a spec using:\n"
" from snakebids import set_bids_spec\n"
f' set_bids_spec("{specs.LATEST}")'
)
warnings.warn(wrn_msg)

if datatype:
path_parts.append(datatype)
path_parts.append(
Expand Down
45 changes: 43 additions & 2 deletions snakebids/tests/test_paths/test_specs.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
from snakebids.paths import specs
from snakebids.paths._config import set_bids_spec
import warnings

import pytest
from pytest_mock import MockerFixture

from snakebids.paths import bids_factory, specs
from snakebids.paths._config import reset_bids_spec, set_bids_spec
from snakebids.paths._presets import bids
from snakebids.paths._utils import find_entity
from snakebids.paths.specs import v0_0_0
Expand Down Expand Up @@ -35,3 +40,39 @@ def test_spec_can_be_set_with_obj():
assert bids(acquisition="foo") == "acquisition-foo"
set_bids_spec(specs.v0_10_1())
assert bids(acquisition="foo") == "acq-foo"


def test_using_include_subject_dir_raises_warning():
with pytest.warns(UserWarning, match="include_session_dir and include_subject_dir"):
bids(subject="001", include_subject_dir=False)
with pytest.warns(UserWarning, match="include_session_dir and include_subject_dir"):
bids(session="001", include_session_dir=False)


class TestCustomEntityWarnings:
def test_using_custom_entities_with_default_bids_raises_warning(self):
reset_bids_spec()
with pytest.warns(UserWarning, match="spec has not been explicitly declared"):
bids(foo="bar")

def test_no_warning_when_spec_declared(self):
reset_bids_spec()
set_bids_spec("v0_0_0")
with warnings.catch_warnings():
warnings.simplefilter("error")
bids(foo="bar")

def test_no_warning_when_bids_explicitly_generated(self):
reset_bids_spec()
with warnings.catch_warnings():
warnings.simplefilter("error")
bids_factory(specs.v0_0_0())(foo="bar")

def test_no_warning_in_interactive_mode(self, mocker: MockerFixture):
reset_bids_spec()
mocker.patch(
"snakebids.paths._factory.in_interactive_session", return_value=True
)
with warnings.catch_warnings():
warnings.simplefilter("error")
bids(foo="bar")

0 comments on commit eab98c7

Please sign in to comment.