Skip to content

Commit

Permalink
Update dependencies and pyright version
Browse files Browse the repository at this point in the history
Fix a number of type issues arising with latest pyright

Need to lock pyright to >=1.1.324 because previous versions have a false
positive bug

Some dict.get(..., {}) require laborious workarounds because the value
type of the dict is a TypedDict, which is different than a regular dict
and can't be constructed easily on the fly. The latest pyright version
uses the latest typeshed, which changed the inference rules to prevent
our previous "hack" from working. This situation will probably never
change.

Broadened the annotated types for some keyword args on functions
accepting both fixed and generic kwargs with differing types. The fixed
kwargs need to allow at least the same types as the generic args, or we
get type issues downstream. Use type validation to compensate
  • Loading branch information
pvandyken committed Aug 24, 2023
1 parent 8d3519e commit 919aae0
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 99 deletions.
50 changes: 25 additions & 25 deletions poetry.lock

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

6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ black = "^23.1.0"
pytest = "^7.0.0"
pytest-mock = "^3.7.0"
isort = "^5.10.1"
poethepoet = "^0.21.1"
poethepoet = "^0.22.0"
pre-commit = "^3.0.0"
mkinit = "^1.0.0"
hypothesis = "^6.34.1"
Expand All @@ -79,9 +79,9 @@ pyfakefs = "^5.1.0"
pyparsing = "^3.0.9"
# Version 1.1.312-1.1.315 have a false positive handling some nested function
# calls
pyright = ">=1.1.307,<1.1.312"
ruff = "^0.0.280"
pathvalidate = "^3.0.0"
pyright = ">=1.1.324"
ruff = "^0.0.285"

[tool.poetry.scripts]
snakebids = "snakebids.admin:main"
Expand Down
17 changes: 10 additions & 7 deletions snakebids/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import pathlib
import re
from collections.abc import Sequence
from typing import Any, Iterable, Mapping, TypeVar, overload
from typing import Any, Mapping, TypeVar, overload

import attr
import snakemake
Expand Down Expand Up @@ -204,7 +204,7 @@ def create_parser(include_snakemake: bool = False) -> argparse.ArgumentParser:

def add_dynamic_args(
parser: argparse.ArgumentParser,
parse_args: Mapping[str, Mapping[str, str | Iterable[str] | bool]],
parse_args: Mapping[str, Any],
pybids_inputs: InputsConfig,
) -> None:
# create parser group for app options
Expand All @@ -216,16 +216,19 @@ def add_dynamic_args(
# We first check that the type annotation is, in fact,
# a str to allow the edge case where it's already
# been converted
arg_copy = dict(arg)
if "type" in arg:
try:
arg_copy["type"] = globals()[str(arg["type"])]
arg_dict = {**arg, "type": globals()[str(arg["type"])]}
except KeyError as err:
raise TypeError(
f"{arg['type']} is not available " + f"as a type for {name}"
f"{arg['type']} is not available as a type for {name}"
) from err

app_group.add_argument(*_make_underscore_dash_aliases(name), **arg_copy)
else:
arg_dict = arg
app_group.add_argument(
*_make_underscore_dash_aliases(name),
**arg_dict,
)

# general parser for
# --filter_{input_type} {key1}={value1} {key2}={value2}...
Expand Down
99 changes: 57 additions & 42 deletions snakebids/core/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from math import inf
from pathlib import Path
from string import Formatter
from typing import Any, Iterable, NoReturn, Sequence, overload
from typing import Any, Iterable, NoReturn, overload

import attr
import more_itertools as itx
Expand Down Expand Up @@ -98,7 +98,7 @@ def expand(
self,
paths: Iterable[Path | str] | Path | str,
/,
allow_missing: bool = False,
allow_missing: bool | str | Iterable[str] = False,
**wildcards: str | Iterable[str],
) -> list[str]:
"""Safely expand over given paths with component wildcards
Expand Down Expand Up @@ -128,7 +128,9 @@ def expand(
"""
return sn_expand(
list(itx.always_iterable(paths)),
allow_missing=allow_missing,
allow_missing=allow_missing
if isinstance(allow_missing, bool)
else list(itx.always_iterable(allow_missing)),
**{self.entity: list(set(self._data))},
**{
wildcard: list(itx.always_iterable(v))
Expand All @@ -141,7 +143,7 @@ def filter(
spec: str | Iterable[str] | None = None,
/,
*,
regex_search: bool = False,
regex_search: bool | str | Iterable[str] = False,
**filters: str | Iterable[str],
) -> Self:
"""Filter component based on provided entity filters
Expand Down Expand Up @@ -170,6 +172,9 @@ def filter(
:attr:`~snakebids.BidsComponentRow.entity` of the
:class:`~snakebids.BidsComponentRow`; all others will be ignored.
"""
if not isinstance(regex_search, bool):
msg = "regex_search must be a boolean"
raise TypeError(msg)
if spec is not None:
if filters:
raise ValueError(
Expand Down Expand Up @@ -208,16 +213,15 @@ class BidsPartialComponent:
``BidsPartialComponents`` are immutable: their values cannot be altered.
"""

zip_lists: ZipList = attr.field(
on_setattr=attr.setters.frozen, converter=MultiSelectDict
_zip_lists: ZipList = attr.field(
on_setattr=attr.setters.frozen, converter=MultiSelectDict, alias="zip_lists"
)
"""Table of unique wildcard groupings for each member in the component.

Dictionary where each key is a wildcard entity and each value is a list of the
values found for that entity. Each of these lists has length equal to the number
of images matched for this modality, so they can be zipped together to get a
list of the wildcard values for each file.
"""
@_zip_lists.validator # type: ignore
def _validate_zip_lists(self, __attr: str, value: dict[str, list[str]]) -> None:
lengths = {len(val) for val in value.values()}
if len(lengths) > 1:
raise ValueError("zip_lists must all be of equal length")

def __repr__(self) -> str:
return self.pformat()
Expand Down Expand Up @@ -274,12 +278,6 @@ def pformat(self, max_width: int | float | None = None, tabstop: int = 4) -> str
]
return "\n".join(output)

@zip_lists.validator # type: ignore
def _validate_zip_lists(self, __attr: str, value: dict[str, list[str]]) -> None:
lengths = {len(val) for val in value.values()}
if len(lengths) > 1:
raise ValueError("zip_lists must all be of equal length")

# Note: we can't use cached property here because it's incompatible with slots.
_input_lists: MultiSelectDict[str, list[str]] | None = attr.field(
default=None, init=False, eq=False, repr=False
Expand All @@ -291,6 +289,17 @@ def _validate_zip_lists(self, __attr: str, value: dict[str, list[str]]) -> None:
default=None, init=False, eq=False, repr=False
)

@property
def zip_lists(self):
"""Table of unique wildcard groupings for each member in the component.
Dictionary where each key is a wildcard entity and each value is a list of the
values found for that entity. Each of these lists has length equal to the number
of images matched for this modality, so they can be zipped together to get a
list of the wildcard values for each file.
"""
return self._zip_lists

@property
def entities(self) -> MultiSelectDict[str, list[str]]:
"""Component entities and their associated values
Expand Down Expand Up @@ -344,7 +353,7 @@ def expand(
self,
paths: Iterable[Path | str] | Path | str,
/,
allow_missing: bool = False,
allow_missing: bool | str | Iterable[str] = False,
**wildcards: str | Iterable[str],
) -> list[str]:
"""Safely expand over given paths with component wildcards
Expand Down Expand Up @@ -372,12 +381,19 @@ def expand(
Keywords not found in the path will be ignored. Keywords take values or
lists of values to be expanded over the provided paths.
"""

def sequencify(item: bool | str | Iterable[str]) -> bool | list[str]:
if isinstance(item, bool):
return item
return list(itx.always_iterable(item))

allow_missing_seq = sequencify(allow_missing)
inner_expand = list(
set(
sn_expand(
list(itx.always_iterable(paths)),
zip,
allow_missing=True if wildcards else allow_missing,
allow_missing=True if wildcards else allow_missing_seq,
**self.zip_lists,
)
)
Expand All @@ -387,7 +403,7 @@ def expand(

return sn_expand(
inner_expand,
allow_missing=allow_missing,
allow_missing=allow_missing_seq,
# Turn all the wildcard items into lists because Snakemake doesn't handle
# iterables very well
**{
Expand All @@ -397,7 +413,10 @@ def expand(
)

def filter(
self, *, regex_search: bool = False, **filters: str | Sequence[str]
self,
*,
regex_search: bool | str | Iterable[str] = False,
**filters: str | Iterable[str],
) -> Self:
"""Filter component based on provided entity filters
Expand All @@ -424,6 +443,9 @@ def filter(
values to be matched with the component
:attr:`~snakebids.BidsComponent.zip_lists`
"""
if not isinstance(regex_search, bool):
msg = "regex_search must be a boolean"
raise TypeError(msg)
return attr.evolve(
self,
zip_lists=filter_list(self.zip_lists, filters, regex_search=regex_search),
Expand Down Expand Up @@ -471,27 +493,11 @@ class BidsComponent(BidsPartialComponent):
path: str = attr.field(on_setattr=attr.setters.frozen)
"""Wildcard-filled path that matches the files for this component."""

zip_lists: ZipList = attr.field(
on_setattr=attr.setters.frozen, converter=MultiSelectDict
_zip_lists: ZipList = attr.field(
on_setattr=attr.setters.frozen, converter=MultiSelectDict, alias="zip_lists"
)
"""Table of unique wildcard groupings for each member in the component.
Dictionary where each key is a wildcard entity and each value is a list of the
values found for that entity. Each of these lists has length equal to the number
of images matched for this modality, so they can be zipped together to get a
list of the wildcard values for each file.
"""

def __repr__(self) -> str:
return self.pformat()

def _pformat_body(self):
return [
f"name={quote_wrap(self.name)},",
f"path={quote_wrap(self.path)},",
]

@zip_lists.validator # type: ignore
@_zip_lists.validator # type: ignore
def _validate_zip_lists(self, __attr: str, value: dict[str, list[str]]) -> None:
super()._validate_zip_lists(__attr, value)
_, raw_fields, *_ = sb_it.unpack(
Expand All @@ -503,6 +509,15 @@ def _validate_zip_lists(self, __attr: str, value: dict[str, list[str]]) -> None:
f"{self.path}: {fields} != zip_lists: {set(value)}"
)

def __repr__(self) -> str:
return self.pformat()

def _pformat_body(self):
return [
f"name={quote_wrap(self.name)},",
f"path={quote_wrap(self.path)},",
]

def __eq__(self, other: BidsComponent | object) -> bool:
if not isinstance(other, self.__class__):
return False
Expand All @@ -519,7 +534,7 @@ def expand(
self,
paths: Iterable[Path | str] | Path | str | None = None,
/,
allow_missing: bool = False,
allow_missing: bool | str | Iterable[str] = False,
**wildcards: str | Iterable[str],
) -> list[str]:
"""Safely expand over given paths with component wildcards
Expand Down
Loading

0 comments on commit 919aae0

Please sign in to comment.