[amas0]

Submission Checklist

• Run unit tests
• Declare copyright holder and open-source license: see below

Summary

Implements a decorator that deprecates use of extra positional arguments for sample, optimize, variational, and generate quantities.

Draft because still need to update tests and docs. Also need to make a decision on whether previous_fit should be allowable as a positional arg for generate_quantities, here I do allow it since it's a core input, but could go either way.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): myself

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)

[amas0]

@WardBrian Got an initial draft implementation to see if this captures the general idea? If it makes sense I'll go ahead and add some tests for it and I suppose we should also add a section in the docs for it.

[WardBrian]

This is more or less what I was hoping for, and in a quick local test it seems to work. Thanks!

Two things:

1. I think besides the existing methods, probably makes sense to do this to install_cmdstan() as well, for everything other than version IMO
2. For other deprecations we havent been using FutureWarning, but rather something like

            get_logger().warning(
                "Argument name `mcmc_sample` is deprecated, please "
                "rename to `previous_fit`."
            )

   I'm not strongly wedded to either, but it makes sense to be consistent?

[amas0]

Ah yeah, better to be consistent on the warning. I think if this were the first time we were doing this, I'd advocate for FutureWarning, but not strongly enough to break from the pattern -- will update. And I'll add those other functions.

[amas0]

Made those updates. Also I had to update a pinned pre-commit version for flake8 to get it to pass, this branch has older ones than develop.

Also, I had to move the deprecation decorator to its own file so it could cleanly be handled in both model and install_cmdstan. Figured since this won't live on past this 1.x version, it's not that big of a deal.

[WardBrian]

Because we from .utils import install_cmdstan in __init__.py, I don't think that module is available to the outside world, which makes it difficult to mock a member of

[amas0]

Mmmmmm. Looks like mock.patch behavior changed in 3.11 which explains the 3.10 failure where the others succeed.

We could just not worry about testing that bit?

[WardBrian]

could also skipif based on sys.version, but I'm happy either way

[amas0]

Marked as ready for review. Did you have any thoughts on documentation? Could just include a note in the 1.x release notes for this

[WardBrian]

Thank you!