-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Refactor internal sampling functions #6444
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
Refactor internal sampling functions #6444
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6444 +/- ##
===========================================
- Coverage 85.92% 41.70% -44.22%
===========================================
Files 148 148
Lines 27691 27668 -23
===========================================
- Hits 23794 11540 -12254
- Misses 3897 16128 +12231
|
356718a
to
d39b685
Compare
52d88e2
to
da79e78
Compare
da79e78
to
af6a52c
Compare
sample_args = { | ||
"draws": draws, | ||
"step": step, | ||
"start": initial_points, | ||
"trace": trace, | ||
"traces": traces, |
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.
Is potential breakage worth changing the arg to plural?
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.
It's a signature change regardless: Optional[BaseTrace]
→ Sequence[BaseTrace]
for multiple functions that this dict is fed to.
All of these are internal API though, so breakage should not be a problem
ff5a6e9
to
2ab5264
Compare
This is a refactor to simplify signatures of internal sampling functions.
The goal is to avoid returning/yielding
BaseTrace
orMultiTrace
objects, making it easier to refactor the trace backend.Checklist
Major / Breaking Changes
pm.iter_sample
function was removed.Maintenance