-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Move sampling code to a submodule #6268
Conversation
4baf3cf
to
84fac13
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6268 +/- ##
==========================================
+ Coverage 94.19% 94.38% +0.19%
==========================================
Files 102 108 +6
Lines 21480 23829 +2349
==========================================
+ Hits 20233 22491 +2258
- Misses 1247 1338 +91
|
837bbfc
to
14bf341
Compare
14bf341
to
2933312
Compare
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 only have two comments:
- There needs to be an empty
__init__.py
file insidetests/sampling
- There are a few places, mainly in the tests, where you import from
pm.sampling.mcmc
orpm.sampling.forward
instead of justpm.sampling
. This is not needed for the few items that are mentioned in__all__
of these two submodules, because they are reimported inpm.sampling
. But it works this way too, I didn't know if you actually wanted to be more precise, and I don't have a strong opinion on this question.
thanks, @Armavica ! I'll add the init file. @ricardoV94 want to add something too before I make the "apply review feedback commit"? |
This is a follow-up to pymc-devs#6257 where we split the `sampling.py` into two files.
This is a separate commit to make sure that git tracks the rename of the old `sampling_jax.py` to `sampling/jax.py` correctly.
2933312
to
f18e61c
Compare
LGTM, do you want a review already or still need to merge the suggestion? |
I already added the |
What is this PR about?
In #6257 we already wanted to do this, but had no good way to keep the
from pymc import sampling_jax
imports working.Today I found a simple solution for this, so this PR moves all
*sampling*.py
files into asampling
submodule.I also moved the tests accordingly.
⚠ The commits must not be squashed, otherwise the git history of
jax.py
would get messed-up. ⚠You can check the diff commit-by-commit to see that I didn't actually change +721-680 lines...
Checklist
Major / Breaking Changes
Bugfixes / New features
Docs / Maintenance
pymc.sampling
submodule to facilitate further refactoring.