math icon indicating copy to clipboard operation
math copied to clipboard

Remove deprecated _log() probability functions. (Breaking change; bump major version)

Open syclik opened this issue 2 years ago • 3 comments

Summary

This PR removes all the deprecated _log() probability functions.

As a consequence, many of the tests needed to be updated.

Tests

I tried to catch all the tests and update them appropiately.

All the log_matches_lpdf and log_matches_lpmf tests are now gone!

Side Effects

This is a breaking change since functions that existed are now removed.

Release notes

Removing deprecated _log distribution functions. Please use _lpdf or _lpmf functions instead.

Checklist

  • [x] Copyright holder: Daniel Lee

    The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses: - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause) - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

  • [ ] the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • [x] the code is written in idiomatic C++ and changes are documented in the doxygen

  • [x] the new changes are tested

syclik avatar Jan 03 '24 06:01 syclik

I'm for this but let's merge it after the current release in two weeks so we have a cycle to see if anything breaks (though I think this should be a fine change)

SteveBronder avatar Jan 03 '24 20:01 SteveBronder

Absolutely! There's no rush. I just had time and wanted to work on it. I'm relying on the CI machinery.

syclik avatar Jan 03 '24 20:01 syclik

(It should definitely go after the release is done.)

syclik avatar Jan 03 '24 22:01 syclik

Once the merge conflict is resolved I'm cool approving this and merging it to the 5.0 branch.

Once the 5.0 branch has another PR in it let's open a PR up for it so the tests go off for the 5.0 branch

SteveBronder avatar Apr 02 '24 16:04 SteveBronder

Just fixed the merge conflict.

The 5.0 branch doesn't have continuous integration in place, unfortunately.

syclik avatar Apr 15 '24 13:04 syclik

Lets start merging to it and open a PR for the 5.0 branch

SteveBronder avatar Apr 15 '24 19:04 SteveBronder