pymc
pymc copied to clipboard
Refactor `get_tau_sigma` and support lists of variables
What is this PR about?
This PR refactors the get_tau_sigma logic in order to support lists of Variables and fixes https://github.com/pymc-devs/pymc/issues/6987.
N.B. The original code had a positivity check for non-Variable arguments, but not for Variable arguments. Since this PR casts the arguments to Variable, I've removed the positivity check for non-Variable arguments.
Checklist
- [X] Explain important implementation details 👆
- [X] Make sure that the pre-commit linting/style checks pass.
- [X] Link relevant issues (preferably in nice commit messages)
- [X] Are the changes covered by tests and docstrings?
- [X] Fill out the short summary sections 👇
Major / Breaking Changes
get_tau_sigmanow always returns aVariable, even when scalars are passed
New features
- N/A
Bugfixes
- https://github.com/pymc-devs/pymc/issues/6987
Documentation
- N/A
Maintenance
- Updated tests
:books: Documentation preview :books:: https://pymc--6988.org.readthedocs.build/en/6988/
Codecov Report
Attention: Patch coverage is 90.00000% with 1 lines in your changes are missing coverage. Please review.
Project coverage is 77.73%. Comparing base (
46f1675) to head (bad3b80). Report is 5 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #6988 +/- ##
===========================================
- Coverage 89.60% 77.73% -11.88%
===========================================
Files 100 100
Lines 16888 16880 -8
===========================================
- Hits 15133 13121 -2012
- Misses 1755 3759 +2004
| Files | Coverage Δ | |
|---|---|---|
| pymc/distributions/shape_utils.py | 59.89% <ø> (-36.05%) |
:arrow_down: |
| pymc/model/core.py | 72.15% <ø> (-19.18%) |
:arrow_down: |
| pymc/model_graph.py | 14.57% <ø> (-62.82%) |
:arrow_down: |
| pymc/pytensorf.py | 73.61% <ø> (-17.42%) |
:arrow_down: |
| pymc/distributions/continuous.py | 64.64% <90.00%> (-32.90%) |
:arrow_down: |
@tvwenger would you like to update this PR? Would only need to add some regression tests for the original issue
@ricardoV94 Thanks for the reminder and sorry for the delay. I added some regression tests, so this should be good to go once the checks are finished.
@ricardoV94 This PR is ready for review
@ricardoV94 Ready for review when checks are done