Remove size from Distribution signatures #5788
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #5788 +/- ##
==========================================
+ Coverage 86.73% 89.36% +2.62%
==========================================
Files 74 74
Lines 13815 13805 -10
==========================================
+ Hits 11983 12337 +354
+ Misses 1832 1468 -364
|
Member
|
Thanks @michaelosthege for taking action! The code here looks good to me, will let others comment on the "philosophy of it" |
Member
|
Related to #5789 |
ricardoV94
reviewed
May 21, 2022
Co-authored-by: Ricardo Vieira <[email protected]>
Co-authored-by: Ricardo Vieira <[email protected]>
Member
Author
|
@lucianopaz @ricardoV94 @twiecki @canyon289 are we happy? (Want to get the ticket off my mental ToDo list..) |
Member
|
I am okay. This does not really address the bigger discussion, but we can definitely merge. |
ricardoV94
approved these changes
May 23, 2022
Member
|
yes, this does two very important thing which is remove the confusion for
users and subset the discussion to an internal one.
thanks much Michael for your patience with all this 🙏
…On Monday, May 23, 2022, Thomas Wiecki ***@***.***> wrote:
Merged #5788 <#5788> into main.
—
Reply to this email directly, view it on GitHub
<#5788 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABXBFYKYD5LQR5WOCWU47BTVLOEIFANCNFSM5WOY4JTA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements how I propose to resolve #5754.
It kicks the argument from the signature, but continues to support it for internal use & to maintain API continuity with Aesara & aeppl.
I made the corresponding changes for
Flat,HalfFlat, but I don't know how to go aboutBoundwhich has multiple other kwargs from the Distribution API in it's signature.Also the GaussianRandomWalk has a
sizekwarg that I wasn't sure how to remove without having to starting thinking.Anybody please feel free to take over the branch.