-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add HalfStudentT moment #5152
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
Add HalfStudentT moment #5152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening the PR. I left some comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There we still some issues in the get_moment. Let me know if the suggestion makes sense
Co-authored-by: Ricardo Vieira <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, waiting to see if tests pass
Pre-commit was failing due to sorting of imports. There is a guide here on how to setup locally to make sure it's working before pushing: https://github.com/pymc-devs/pymc/wiki/Python-Code-Style |
Codecov Report
@@ Coverage Diff @@
## main #5152 +/- ##
==========================================
+ Coverage 77.86% 78.00% +0.14%
==========================================
Files 88 88
Lines 14165 14114 -51
==========================================
- Hits 11029 11010 -19
+ Misses 3136 3104 -32
|
(1, 1, 5, np.ones(5)), | ||
(1, np.arange(5), None, np.arange(5)), | ||
(1, np.arange(5), (2, 5), np.full((2, 5), np.arange(5))), | ||
(np.arange(1, 6), 1, 5, np.full(5, 1)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one does not really test the broadcast because size is not None. Maybe something like this?
(np.arange(1, 6), 1, 5, np.full(5, 1)), | |
(np.arange(1, 6), 1, None, np.full(5, 1)), |
And you can remove one of the previous 2 tests, we don't need 5 in total.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Thanks for your patience!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thank you :)
Thanks! |
Add moments and tests for the below distribution as part of #5078
pymc.distributions.continuous.HalfStudentT