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

Deprecate positional boolean arguments #1694

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion .azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ trigger:

variables:
RUN_COVERAGE: no
PYTEST_ADDOPTS: --color=yes --junitxml=test-data/test-results.xml
PYTEST_ADDOPTS: -v --color=yes --junitxml=test-data/test-results.xml
DEPENDENCIES_VERSION: "latest" # |"pre-release" | "minimum-version"
TEST_TYPE: "standard" # | "coverage"

Expand Down
10 changes: 8 additions & 2 deletions benchmarks/benchmarks/anndata.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ def track_peakmem_garbage_collection(self, *_):
def display_top(snapshot, key_type="lineno"):
snapshot = snapshot.filter_traces(
(
tracemalloc.Filter(False, "<frozen importlib._bootstrap>"),
tracemalloc.Filter(False, "<unknown>"),
tracemalloc.Filter(
inclusive=False,
filename_pattern="<frozen importlib._bootstrap>",
),
tracemalloc.Filter(
inclusive=False,
filename_pattern="<unknown>",
),
)
)
top_stats = snapshot.statistics(key_type)
Expand Down
3 changes: 2 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@
# Generate the API documentation when building
autosummary_generate = True
autodoc_member_order = "bysource"
autodoc_mock_imports = ["torch"]
# autodoc_default_flags = ['members']
issues_github_path = "scverse/anndata"
rtd_links_prefix = PurePosixPath("src")
# autodoc_default_flags = ['members']
napoleon_google_docstring = False
napoleon_numpy_docstring = True
napoleon_include_init_with_doc = False
Expand Down
2 changes: 1 addition & 1 deletion docs/extensions/no_skip_abc_members.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def autodoc_skip_member(
what: Literal["module", "class", "exception", "function", "method", "attribute"],
name: str,
obj: object,
skip: bool,
skip: bool, # noqa: FBT001
options: Options,
):
if what == "method" and getattr(obj, "__isabstractmethod__", False):
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ dependencies = [
"packaging>=20.0",
# array-api-compat 1.5 has https://github.com/scverse/anndata/issues/1410
"array_api_compat>1.4,!=1.5",
"legacy-api-wrap",
]
dynamic = ["version"]

Expand Down Expand Up @@ -172,6 +173,7 @@ docstring-code-format = true
select = [
"E", # Error detected by Pycodestyle
"F", # Errors detected by Pyflakes
"FBT", # Boolean positional arguments
"W", # Warning detected by Pycodestyle
"PLW", # Pylint
"UP", # pyupgrade
Expand Down Expand Up @@ -204,6 +206,7 @@ required-imports = ["from __future__ import annotations"]
"subprocess.call".msg = "Use `subprocess.run([…])` instead"
"subprocess.check_call".msg = "Use `subprocess.run([…], check=True)` instead"
"subprocess.check_output".msg = "Use `subprocess.run([…], check=True, capture_output=True)` instead"
"legacy_api_wrap.legacy_api".msg = "Use anndata.compat.old_positionals instead"
[tool.ruff.lint.flake8-type-checking]
exempt-modules = []
strict = true
Expand Down
51 changes: 35 additions & 16 deletions src/anndata/_core/anndata.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

from .. import utils
from .._settings import settings
from ..compat import DaskArray, SpArray, ZarrArray, _move_adj_mtx
from ..compat import DaskArray, SpArray, ZarrArray, _move_adj_mtx, old_positionals
from ..logging import anndata_logger as logger
from ..utils import (
axis_len,
Expand Down Expand Up @@ -218,12 +218,24 @@ class AnnData(metaclass=utils.DeprecationMixinMeta):
var={"var_names", "col_names", "index"},
)

@old_positionals(
"obsm",
"varm",
"layers",
"raw",
"dtype",
"shape",
"filename",
"filemode",
"asview",
)
def __init__(
self,
X: np.ndarray | sparse.spmatrix | pd.DataFrame | None = None,
obs: pd.DataFrame | Mapping[str, Iterable[Any]] | None = None,
var: pd.DataFrame | Mapping[str, Iterable[Any]] | None = None,
uns: Mapping[str, Any] | None = None,
*,
obsm: np.ndarray | Mapping[str, Sequence[Any]] | None = None,
varm: np.ndarray | Mapping[str, Sequence[Any]] | None = None,
layers: Mapping[str, np.ndarray | sparse.spmatrix] | None = None,
Expand All @@ -233,7 +245,6 @@ def __init__(
filename: PathLike | None = None,
filemode: Literal["r", "r+"] | None = None,
asview: bool = False,
*,
obsp: np.ndarray | Mapping[str, Sequence[Any]] | None = None,
varp: np.ndarray | Mapping[str, Sequence[Any]] | None = None,
oidx: Index1D | None = None,
Expand Down Expand Up @@ -469,7 +480,10 @@ def _init_as_actual(
# layers
self.layers = layers

def __sizeof__(self, show_stratified=None, with_disk: bool = False) -> int:
@old_positionals("show_stratified", "with_disk")
def __sizeof__(
self, *, show_stratified: bool = False, with_disk: bool = False
) -> int:
def get_size(X) -> int:
def cs_to_bytes(X) -> int:
return int(X.data.nbytes + X.indptr.nbytes + X.indices.nbytes)
Expand Down Expand Up @@ -1248,7 +1262,7 @@ def to_df(self, layer: str | None = None) -> pd.DataFrame:
X = X.toarray()
return pd.DataFrame(X, index=self.obs_names, columns=self.var_names)

def _get_X(self, use_raw=False, layer=None):
def _get_X(self, *, use_raw: bool = False, layer: str | None = None):
"""\
Convenience method for getting expression values
with common arguments and error handling.
Expand Down Expand Up @@ -1332,8 +1346,8 @@ def var_vector(self, k, *, layer: str | None = None) -> np.ndarray:
layer = None
return get_vector(self, k, "var", "obs", layer=layer)

@utils.deprecated("obs_vector")
def _get_obs_array(self, k, use_raw=False, layer=None):
@deprecated("obs_vector")
def _get_obs_array(self, k, use_raw=False, layer=None): # noqa: FBT002
"""\
Get an array from the layer (default layer='X') along the :attr:`obs`
dimension by first looking up `obs.keys` and then :attr:`obs_names`.
Expand All @@ -1343,8 +1357,8 @@ def _get_obs_array(self, k, use_raw=False, layer=None):
else:
return self.raw.obs_vector(k)

@utils.deprecated("var_vector")
def _get_var_array(self, k, use_raw=False, layer=None):
@deprecated("var_vector")
def _get_var_array(self, k, use_raw=False, layer=None): # noqa: FBT002
"""\
Get an array from the layer (default layer='X') along the :attr:`var`
dimension by first looking up `var.keys` and then :attr:`var_names`.
Expand Down Expand Up @@ -1383,7 +1397,8 @@ def _mutated_copy(self, **kwargs):
new["raw"] = self.raw.copy()
return AnnData(**new)

def to_memory(self, copy=False) -> AnnData:
@old_positionals("copy")
def to_memory(self, *, copy: bool = False) -> AnnData:
"""Return a new AnnData object with all backed arrays loaded into memory.

Params
Expand Down Expand Up @@ -1414,13 +1429,13 @@ def to_memory(self, copy=False) -> AnnData:
]:
attr = getattr(self, attr_name, None)
if attr is not None:
new[attr_name] = to_memory(attr, copy)
new[attr_name] = to_memory(attr, copy=copy)

if self.raw is not None:
new["raw"] = {
"X": to_memory(self.raw.X, copy),
"var": to_memory(self.raw.var, copy),
"varm": to_memory(self.raw.varm, copy),
"X": to_memory(self.raw.X, copy=copy),
"var": to_memory(self.raw.var, copy=copy),
"varm": to_memory(self.raw.varm, copy=copy),
}

if self.isbacked:
Expand Down Expand Up @@ -1876,7 +1891,8 @@ def write_h5ad(

write = write_h5ad # a shortcut and backwards compat

def write_csvs(self, dirname: PathLike, skip_data: bool = True, sep: str = ","):
@old_positionals("skip_data", "sep")
def write_csvs(self, dirname: PathLike, *, skip_data: bool = True, sep: str = ","):
"""\
Write annotation to `.csv` files.

Expand All @@ -1896,7 +1912,8 @@ def write_csvs(self, dirname: PathLike, skip_data: bool = True, sep: str = ","):

write_csvs(dirname, self, skip_data=skip_data, sep=sep)

def write_loom(self, filename: PathLike, write_obsm_varm: bool = False):
@old_positionals("write_obsm_varm")
def write_loom(self, filename: PathLike, *, write_obsm_varm: bool = False):
"""\
Write `.loom`-formatted hdf5 file.

Expand Down Expand Up @@ -1949,9 +1966,11 @@ def chunked_X(self, chunk_size: int | None = None):
if start < n:
yield (self.X[start:n], start, n)

@old_positionals("replace")
def chunk_X(
self,
select: int | Sequence[int] | np.ndarray = 1000,
*,
replace: bool = True,
):
"""\
Expand Down Expand Up @@ -2009,7 +2028,7 @@ def _has_X(self) -> bool:
# --------------------------------------------------------------------------

@property
@utils.deprecated("is_view")
@deprecated("is_view")
def isview(self):
return self.is_view

Expand Down
13 changes: 7 additions & 6 deletions src/anndata/_core/file_backing.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from os import PathLike
from typing import Literal

from .._types import ArrayStorageType
from . import anndata


Expand Down Expand Up @@ -118,7 +119,7 @@ def is_open(self) -> bool:


@singledispatch
def to_memory(x, copy=False):
def to_memory(x, *, copy: bool = False):
"""Permissivley convert objects to in-memory representation.

If they already are in-memory, (or are just unrecognized) pass a copy through.
Expand All @@ -131,27 +132,27 @@ def to_memory(x, copy=False):

@to_memory.register(ZarrArray)
@to_memory.register(h5py.Dataset)
def _(x, copy=False):
def _(x: ArrayStorageType, *, copy: bool = False):
return x[...]


@to_memory.register(BaseCompressedSparseDataset)
def _(x: BaseCompressedSparseDataset, copy=True):
def _(x: BaseCompressedSparseDataset, *, copy: bool = False):
return x.to_memory()


@to_memory.register(DaskArray)
def _(x, copy=False):
def _(x: DaskArray, *, copy: bool = False):
return x.compute()


@to_memory.register(Mapping)
def _(x: Mapping, copy=False):
def _(x: Mapping, *, copy: bool = False):
return {k: to_memory(v, copy=copy) for k, v in x.items()}


@to_memory.register(AwkArray)
def _(x, copy=False):
def _(x: AwkArray, *, copy: bool = False):
from copy import copy as _copy

if copy:
Expand Down
2 changes: 1 addition & 1 deletion src/anndata/_core/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def equal_awkward(a, b) -> bool:
return ak.almost_equal(a, b)


def as_sparse(x, use_sparse_array=False):
def as_sparse(x, *, use_sparse_array=False):
if not isinstance(x, sparse.spmatrix | SpArray):
if CAN_USE_SPARSE_ARRAY and use_sparse_array:
return sparse.csr_array(x)
Expand Down
2 changes: 1 addition & 1 deletion src/anndata/_io/specs/methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ def read_series(dataset: h5py.Dataset) -> np.ndarray | pd.Categorical:
parent = dataset.parent
categories_dset = parent[_read_attr(dataset.attrs, "categories")]
categories = read_elem(categories_dset)
ordered = bool(_read_attr(categories_dset.attrs, "ordered", False))
ordered = bool(_read_attr(categories_dset.attrs, "ordered", default=False))
return pd.Categorical.from_codes(
read_elem(dataset), categories, ordered=ordered
)
Expand Down
7 changes: 5 additions & 2 deletions src/anndata/_io/write.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from scipy.sparse import issparse

from .._warnings import WriteWarning
from ..compat import old_positionals
from ..logging import get_logger

if TYPE_CHECKING:
Expand All @@ -21,8 +22,9 @@
logger = get_logger(__name__)


@old_positionals("skip_data", "sep")
def write_csvs(
dirname: PathLike, adata: AnnData, skip_data: bool = True, sep: str = ","
dirname: PathLike, adata: AnnData, *, skip_data: bool = True, sep: str = ","
):
"""See :meth:`~anndata.AnnData.write_csvs`."""
dirname = Path(dirname)
Expand Down Expand Up @@ -75,7 +77,8 @@ def write_csvs(
)


def write_loom(filename: PathLike, adata: AnnData, write_obsm_varm: bool = False):
@old_positionals("write_obsm_varm")
def write_loom(filename: PathLike, adata: AnnData, *, write_obsm_varm: bool = False):
filename = Path(filename)
row_attrs = {k: np.array(v) for k, v in adata.var.to_dict("list").items()}
row_names = adata.var_names
Expand Down
14 changes: 12 additions & 2 deletions src/anndata/compat/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from collections.abc import Mapping
from contextlib import AbstractContextManager
from dataclasses import dataclass, field
from functools import singledispatch, wraps
from functools import partial, singledispatch, wraps
from importlib.util import find_spec
from inspect import Parameter, signature
from pathlib import Path
Expand Down Expand Up @@ -165,6 +165,16 @@ def __repr__():
return "mock cupy.ndarray"


if find_spec("legacy_api_wrap") or TYPE_CHECKING:
from legacy_api_wrap import legacy_api # noqa: TID251

old_positionals = partial(legacy_api, category=FutureWarning)
else:

def old_positionals(*old_positionals):
return lambda func: func


#############################
# IO helpers
#############################
Expand Down Expand Up @@ -231,7 +241,7 @@ def _from_fixed_length_strings(value):


def _decode_structured_array(
arr: np.ndarray, dtype: np.dtype | None = None, copy: bool = False
arr: np.ndarray, *, dtype: np.dtype | None = None, copy: bool = False
) -> np.ndarray:
"""
h5py 3.0 now reads all strings as bytes. There is a helper method which can convert these to strings,
Expand Down
Loading