From 4a98abdb81537f98242588631f5c05c3f0586767 Mon Sep 17 00:00:00 2001 From: amas Date: Mon, 2 Nov 2026 15:30:44 -0500 Subject: [PATCH 1/3] Implement initial positional-arg deprecation decorator --- cmdstanpy/model.py | 64 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index 7e509981..2befdced 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -1,5 +1,7 @@ """CmdStanModel""" +import functools +import inspect import io import os import platform @@ -9,6 +11,7 @@ import sys import tempfile import threading +import warnings from collections import OrderedDict from concurrent.futures import ThreadPoolExecutor from io import StringIO @@ -57,6 +60,55 @@ OptionalPath = Union[str, os.PathLike, None] Fit = TypeVar('Fit', CmdStanMCMC, CmdStanMLE, CmdStanVB) +F = TypeVar('F', bound=Callable[..., Any]) + + +def _deprecate_positional_args(last_deprecated: str) -> Callable[[F], F]: + """ + Allow, with a FutureWarning, keyword-only arguments up to and including + ``last_deprecated`` to still be passed by position. + + Temporary shim for the 1.x series; positional use of these + arguments becomes an error in CmdStanPy 2.0. + """ + + def decorator(func: F) -> F: + params = inspect.signature(func).parameters.values() + n_positional = sum( + p.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD for p in params + ) + kw_only = [ + p.name for p in params if p.kind == inspect.Parameter.KEYWORD_ONLY + ] + deprecated = kw_only[: kw_only.index(last_deprecated) + 1] + + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + extra = args[n_positional:] + if not extra or len(extra) > len(deprecated): + # either nothing to do, or let Python raise its usual error + return func(*args, **kwargs) + names = deprecated[: len(extra)] + for name in names: + if name in kwargs: + raise TypeError( + f"{func.__qualname__}() got multiple values for " + f"argument '{name}'" + ) + warnings.warn( + f"Passing {', '.join(map(repr, names))} to " + f"{func.__qualname__}() by position is deprecated and will " + "raise an error in CmdStanPy 2.0. Pass them as keyword " + f"arguments instead, e.g. {names[0]}=...", + FutureWarning, + stacklevel=2, + ) + kwargs.update(zip(names, extra)) + return func(*args[:n_positional], **kwargs) + + return wrapper # type: ignore[return-value] + + return decorator class CmdStanModel: @@ -304,9 +356,11 @@ def code(self) -> Optional[str]: ) return code + @_deprecate_positional_args(last_deprecated="jacobian") def optimize( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, + *, seed: Optional[int] = None, inits: Union[Mapping[str, Any], float, str, os.PathLike, None] = None, output_dir: OptionalPath = None, @@ -328,7 +382,6 @@ def optimize( time_fmt: str = "%Y%m%d%H%M%S", timeout: Optional[float] = None, jacobian: bool = False, - # would be nice to move this further up, but that's a breaking change ) -> CmdStanMLE: """ Run the specified CmdStan optimize algorithm to produce a @@ -496,9 +549,11 @@ def optimize( return mle # pylint: disable=too-many-arguments + @_deprecate_positional_args(last_deprecated="timeout") def sample( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, + *, chains: Optional[int] = None, parallel_chains: Optional[int] = None, threads_per_chain: Optional[int] = None, @@ -536,7 +591,6 @@ def sample( refresh: Optional[int] = None, time_fmt: str = "%Y%m%d%H%M%S", timeout: Optional[float] = None, - *, force_one_process_per_chain: Optional[bool] = None, inv_metric: Union[ str, @@ -1034,10 +1088,12 @@ def sample( mcmc = CmdStanMCMC(runset) return mcmc + @_deprecate_positional_args(last_deprecated="timeout") def generate_quantities( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, previous_fit: Union[Fit, list[str], None] = None, + *, seed: Optional[int] = None, gq_output_dir: OptionalPath = None, sig_figs: Optional[int] = None, @@ -1045,7 +1101,6 @@ def generate_quantities( refresh: Optional[int] = None, time_fmt: str = "%Y%m%d%H%M%S", timeout: Optional[float] = None, - *, mcmc_sample: Union[CmdStanMCMC, list[str], None] = None, ) -> CmdStanGQ[Fit]: """ @@ -1227,9 +1282,11 @@ def generate_quantities( quantities = CmdStanGQ(runset=runset, previous_fit=fit_object) return quantities + @_deprecate_positional_args(last_deprecated="timeout") def variational( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, + *, seed: Optional[int] = None, inits: Optional[float] = None, output_dir: OptionalPath = None, @@ -1251,7 +1308,6 @@ def variational( refresh: Optional[int] = None, time_fmt: str = "%Y%m%d%H%M%S", timeout: Optional[float] = None, - *, output_samples: Optional[int] = None, save_latent_dynamics: Optional[bool] = None, ) -> CmdStanVB: From 4e0767a3c391c995d7ad9fb7e8c5fdbc905a156c Mon Sep 17 00:00:00 2001 From: amas Date: Mon, 2 Nov 2026 16:10:01 -0500 Subject: [PATCH 2/3] Broaden deprecations and add tests --- .pre-commit-config.yaml | 2 +- cmdstanpy/model.py | 61 ++------------- cmdstanpy/utils/cmdstan.py | 4 +- cmdstanpy/utils/deprecation.py | 58 ++++++++++++++ test/test_deprecations.py | 134 +++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 58 deletions(-) create mode 100644 cmdstanpy/utils/deprecation.py create mode 100644 test/test_deprecations.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c8a219e1..920a990c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,7 +16,7 @@ repos: hooks: - id: black - repo: https://github.com/pycqa/flake8 - rev: 5.0.4 + rev: 7.3.0 hooks: - id: flake8 - repo: https://github.com/pre-commit/mirrors-mypy diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index 2befdced..fdd21798 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -1,7 +1,5 @@ """CmdStanModel""" -import functools -import inspect import io import os import platform @@ -11,7 +9,6 @@ import sys import tempfile import threading -import warnings from collections import OrderedDict from concurrent.futures import ThreadPoolExecutor from io import StringIO @@ -49,6 +46,7 @@ do_command, get_logger, ) +from cmdstanpy.utils.deprecation import deprecate_positional_args from cmdstanpy.utils.filesystem import ( temp_inits, temp_metrics, @@ -60,55 +58,6 @@ OptionalPath = Union[str, os.PathLike, None] Fit = TypeVar('Fit', CmdStanMCMC, CmdStanMLE, CmdStanVB) -F = TypeVar('F', bound=Callable[..., Any]) - - -def _deprecate_positional_args(last_deprecated: str) -> Callable[[F], F]: - """ - Allow, with a FutureWarning, keyword-only arguments up to and including - ``last_deprecated`` to still be passed by position. - - Temporary shim for the 1.x series; positional use of these - arguments becomes an error in CmdStanPy 2.0. - """ - - def decorator(func: F) -> F: - params = inspect.signature(func).parameters.values() - n_positional = sum( - p.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD for p in params - ) - kw_only = [ - p.name for p in params if p.kind == inspect.Parameter.KEYWORD_ONLY - ] - deprecated = kw_only[: kw_only.index(last_deprecated) + 1] - - @functools.wraps(func) - def wrapper(*args: Any, **kwargs: Any) -> Any: - extra = args[n_positional:] - if not extra or len(extra) > len(deprecated): - # either nothing to do, or let Python raise its usual error - return func(*args, **kwargs) - names = deprecated[: len(extra)] - for name in names: - if name in kwargs: - raise TypeError( - f"{func.__qualname__}() got multiple values for " - f"argument '{name}'" - ) - warnings.warn( - f"Passing {', '.join(map(repr, names))} to " - f"{func.__qualname__}() by position is deprecated and will " - "raise an error in CmdStanPy 2.0. Pass them as keyword " - f"arguments instead, e.g. {names[0]}=...", - FutureWarning, - stacklevel=2, - ) - kwargs.update(zip(names, extra)) - return func(*args[:n_positional], **kwargs) - - return wrapper # type: ignore[return-value] - - return decorator class CmdStanModel: @@ -356,7 +305,7 @@ def code(self) -> Optional[str]: ) return code - @_deprecate_positional_args(last_deprecated="jacobian") + @deprecate_positional_args(last_deprecated="jacobian") def optimize( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, @@ -549,7 +498,7 @@ def optimize( return mle # pylint: disable=too-many-arguments - @_deprecate_positional_args(last_deprecated="timeout") + @deprecate_positional_args(last_deprecated="timeout") def sample( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, @@ -1088,7 +1037,7 @@ def sample( mcmc = CmdStanMCMC(runset) return mcmc - @_deprecate_positional_args(last_deprecated="timeout") + @deprecate_positional_args(last_deprecated="timeout") def generate_quantities( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, @@ -1282,7 +1231,7 @@ def generate_quantities( quantities = CmdStanGQ(runset=runset, previous_fit=fit_object) return quantities - @_deprecate_positional_args(last_deprecated="timeout") + @deprecate_positional_args(last_deprecated="timeout") def variational( self, data: Union[Mapping[str, Any], str, os.PathLike, None] = None, diff --git a/cmdstanpy/utils/cmdstan.py b/cmdstanpy/utils/cmdstan.py index 60b6551e..a5848024 100644 --- a/cmdstanpy/utils/cmdstan.py +++ b/cmdstanpy/utils/cmdstan.py @@ -14,6 +14,7 @@ from cmdstanpy import _DOT_CMDSTAN from .. import progress as progbar +from .deprecation import deprecate_positional_args from .logging import get_logger EXTENSION = '.exe' if platform.system() == 'Windows' else '' @@ -433,15 +434,16 @@ def cxx_toolchain_path( return compiler_path, tool_path +@deprecate_positional_args(last_deprecated="cores") def install_cmdstan( version: Optional[str] = None, + *, dir: Optional[str] = None, overwrite: bool = False, compiler: bool = False, progress: bool = False, verbose: bool = False, cores: int = 1, - *, interactive: bool = False, ) -> bool: """ diff --git a/cmdstanpy/utils/deprecation.py b/cmdstanpy/utils/deprecation.py new file mode 100644 index 00000000..55e034b2 --- /dev/null +++ b/cmdstanpy/utils/deprecation.py @@ -0,0 +1,58 @@ +""" +Temporary helpers for deprecations in the 1.x series. +""" + +import functools +import inspect +from typing import Any, Callable, TypeVar + +from .logging import get_logger + +F = TypeVar('F', bound=Callable[..., Any]) + + +def deprecate_positional_args(last_deprecated: str) -> Callable[[F], F]: + """ + Allow keyword-only arguments up to and including ``last_deprecated`` + to still be passed by position, logging a deprecation warning. + + Positional use of these arguments becomes an error in CmdStanPy 2.0. + """ + + def decorator(func: F) -> F: + params = inspect.signature(func).parameters.values() + n_positional = sum( + p.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD for p in params + ) + kw_only = [ + p.name for p in params if p.kind == inspect.Parameter.KEYWORD_ONLY + ] + deprecated = kw_only[: kw_only.index(last_deprecated) + 1] + + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + extra = args[n_positional:] + if not extra or len(extra) > len(deprecated): + # either nothing to do, or let Python raise its usual error + return func(*args, **kwargs) + names = deprecated[: len(extra)] + for name in names: + if name in kwargs: + raise TypeError( + f"{func.__qualname__}() got multiple values for " + f"argument '{name}'" + ) + quoted = ", ".join("`" + name + "`" for name in names) + get_logger().warning( + "Passing %s to %s() by position is deprecated and will raise " + "an error in CmdStanPy 2.0. Please pass them as keyword " + "arguments instead.", + quoted, + func.__qualname__, + ) + kwargs.update(zip(names, extra)) + return func(*args[:n_positional], **kwargs) + + return wrapper # type: ignore[return-value] + + return decorator diff --git a/test/test_deprecations.py b/test/test_deprecations.py new file mode 100644 index 00000000..8fbac2c5 --- /dev/null +++ b/test/test_deprecations.py @@ -0,0 +1,134 @@ +"""Tests for deprecated positional arguments to public functions""" + +# positional calls below are deliberate +# pylint: disable=too-many-function-args + +import inspect +import logging +import os +import re +from test import check_present +from typing import Any, Callable +from unittest import mock + +import pytest + +from cmdstanpy.model import CmdStanModel +from cmdstanpy.utils import install_cmdstan +from cmdstanpy.utils.deprecation import deprecate_positional_args + +HERE = os.path.dirname(os.path.abspath(__file__)) +DATAFILES_PATH = os.path.join(HERE, 'data') + + +class Dummy: + @deprecate_positional_args(last_deprecated="c") + def method( + self, a: Any = None, *, b: Any = None, c: Any = None, d: Any = None + ) -> tuple[Any, Any, Any, Any]: + return a, b, c, d + + +def test_keywords_do_not_warn(caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.WARNING): + assert Dummy().method(1, b=2, c=3, d=4) == (1, 2, 3, 4) + assert Dummy().method() == (None, None, None, None) + assert not caplog.records + + +def test_positional_maps_to_keywords(caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.WARNING): + assert Dummy().method(1, 2, 3, d=4) == (1, 2, 3, 4) + check_present( + caplog, + ( + 'cmdstanpy', + 'WARNING', + re.compile(r"Passing `b`, `c` to Dummy\.method\(\) by position"), + ), + ) + + +def test_positional_and_keyword_collision() -> None: + with pytest.raises(TypeError, match="multiple values for argument 'b'"): + Dummy().method(1, 2, b=3) + + +def test_too_many_positional() -> None: + # `d` was never positional, so it must not be filled by position + with pytest.raises(TypeError): + Dummy().method(1, 2, 3, 4) + + +def test_signature_is_keyword_only() -> None: + params = inspect.signature(Dummy.method).parameters + assert params['a'].kind == inspect.Parameter.POSITIONAL_OR_KEYWORD + assert params['b'].kind == inspect.Parameter.KEYWORD_ONLY + + +@pytest.mark.parametrize( + 'func, positional, last_deprecated', + [ + (CmdStanModel.sample, ['self', 'data'], 'timeout'), + (CmdStanModel.optimize, ['self', 'data'], 'jacobian'), + (CmdStanModel.variational, ['self', 'data'], 'timeout'), + ( + CmdStanModel.generate_quantities, + ['self', 'data', 'previous_fit'], + 'timeout', + ), + (install_cmdstan, ['version'], 'cores'), + ], +) +def test_public_signatures( + func: Callable[..., Any], positional: list[str], last_deprecated: str +) -> None: + params = inspect.signature(func).parameters + kinds = {name: p.kind for name, p in params.items()} + assert [ + n + for n, k in kinds.items() + if k == inspect.Parameter.POSITIONAL_OR_KEYWORD + ] == positional + # guards against a rename silently shifting the positional mapping + assert kinds[last_deprecated] == inspect.Parameter.KEYWORD_ONLY + + +def test_optimize_positional_seed(caplog: pytest.LogCaptureFixture) -> None: + stan = os.path.join(DATAFILES_PATH, 'bernoulli.stan') + jdata = os.path.join(DATAFILES_PATH, 'bernoulli.data.json') + model = CmdStanModel(stan_file=stan) + with caplog.at_level(logging.WARNING): + mle_pos = model.optimize(jdata, 1234) + check_present( + caplog, + ( + 'cmdstanpy', + 'WARNING', + re.compile(r"Passing `seed` to CmdStanModel\.optimize\(\)"), + ), + ) + mle_kw = model.optimize(jdata, seed=1234) + assert mle_pos.metadata.cmdstan_config['seed'] == 1234 + assert mle_pos.optimized_params_dict == mle_kw.optimized_params_dict + + +def test_install_cmdstan_positional(caplog: pytest.LogCaptureFixture) -> None: + # stop right after argument handling; nothing is downloaded + with mock.patch( + 'cmdstanpy.install_cmdstan.run_install', side_effect=RuntimeError + ) as run_install: + with caplog.at_level(logging.WARNING): + install_cmdstan("2.36.0", "some_dir", True) + check_present( + caplog, + ( + 'cmdstanpy', + 'WARNING', + re.compile(r"Passing `dir`, `overwrite` to install_cmdstan\(\)"), + ), + ) + settings = run_install.call_args.args[0] + assert settings.version == "2.36.0" + assert settings.dir == "some_dir" + assert settings.overwrite From ad982578de50d8994365614cf3cecd14e7ad01a2 Mon Sep 17 00:00:00 2001 From: amas Date: Mon, 2 Nov 2026 16:48:58 -0500 Subject: [PATCH 3/3] Skip mock.patch on install_cmdstan in tests in python 3.10 --- test/test_deprecations.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test_deprecations.py b/test/test_deprecations.py index 8fbac2c5..3a7b36e4 100644 --- a/test/test_deprecations.py +++ b/test/test_deprecations.py @@ -7,6 +7,7 @@ import logging import os import re +import sys from test import check_present from typing import Any, Callable from unittest import mock @@ -113,6 +114,11 @@ def test_optimize_positional_seed(caplog: pytest.LogCaptureFixture) -> None: assert mle_pos.optimized_params_dict == mle_kw.optimized_params_dict +@pytest.mark.skipif( + sys.version_info < (3, 11), + reason="mock.patch resolves `cmdstanpy.install_cmdstan` to the " + "re-exported function, not the module, before Python 3.11", +) def test_install_cmdstan_positional(caplog: pytest.LogCaptureFixture) -> None: # stop right after argument handling; nothing is downloaded with mock.patch(