-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Improve sampling coverage #4270
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
Conversation
assert isinstance(result, az.InferenceData) | ||
assert result.posterior.sizes["draw"] == 100 | ||
assert result.posterior.sizes["chain"] == 2 | ||
assert len(result._groups_warmup) == 0 | ||
pass |
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.
unnecessary pass
statement
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 like to put them there to mark the end of a test/function (just like an unnecessary return
).
In my opinion they have a few advantages:
- trailing comments are collapsed together with the code
- because the
pass
in a test, orreturn
in a function was put there on purpose, you don't wonder if somebody just stopped writing, or accidentally deleted some code
Not saying you must put it back in - just showing a maybe new perspective :)
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.
To me that looks weird and confusing.
9b89140
to
3e4d5fa
Compare
3e4d5fa
to
1f2c294
Compare
Codecov Report
@@ Coverage Diff @@
## master #4270 +/- ##
==========================================
+ Coverage 87.44% 87.51% +0.06%
==========================================
Files 88 88
Lines 14342 14333 -9
==========================================
+ Hits 12542 12543 +1
+ Misses 1800 1790 -10
|
assert isinstance(result, az.InferenceData) | ||
assert result.posterior.sizes["draw"] == 100 | ||
assert result.posterior.sizes["chain"] == 2 | ||
assert len(result._groups_warmup) == 0 | ||
pass |
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 like to put them there to mark the end of a test/function (just like an unnecessary return
).
In my opinion they have a few advantages:
- trailing comments are collapsed together with the code
- because the
pass
in a test, orreturn
in a function was put there on purpose, you don't wonder if somebody just stopped writing, or accidentally deleted some code
Not saying you must put it back in - just showing a maybe new perspective :)
Looks generally good! Just a few questions/details in the comments and then we'll be ✔️ |
Thanks @MarcoGorelli! |
Added test cases for
pm.sample(idata_kwargs=....)
and forrandom_seed=-1
.While doing this, I also came across some outdated Python2 constructs (which I changed)