pymc
pymc copied to clipboard
Update docstrings for sample_smc and smc.py
What is this PR about? #DataUmbrellaPyMCSprint References #5459
- Docstring formatting
- Some typos
- Added in kwargs for IMH and MH
- Unsure about line “Model (optional if in
with
context)” but I saw this was still present on several other pages that had already been edited - I think there may be more in here that are optional?
- For random_seed (int, array_like of int, RandomState or Generator, optional) unsure if you want curly bracket format like in numpydoc - didn’t see that used anywhere else in pymc docs i.e. in NumPy docs seed{None, int, array_like[ints], SeedSequence, BitGenerator, Generator}, optional
- Was unable to get sphinx preview working when using virtual env so I hope this is ok
Checklist
- [ ] Explain important implementation details 👆
- [ ] Make sure that the pre-commit linting/style checks pass.
- [ ] Link relevant issues (preferably in nice commit messages)
- [ ] Are the changes covered by tests and docstrings?
- [ ] Fill out the short summary sections 👇
Major / Breaking Changes
- ...
Bugfixes / New features
- ...
Docs / Maintenance
- ...
Codecov Report
Merging #6114 (9b72724) into main (4acd98e) will not change coverage. The diff coverage is
n/a
.
Additional details and impacted files
@@ Coverage Diff @@
## main #6114 +/- ##
=======================================
Coverage 94.17% 94.17%
=======================================
Files 111 111
Lines 23908 23908
=======================================
Hits 22515 22515
Misses 1393 1393
Impacted Files | Coverage Δ | |
---|---|---|
pymc/smc/kernels.py | 97.41% <ø> (ø) |
|
pymc/smc/sampling.py | 86.42% <ø> (ø) |
|
pymc/variational/inference.py | 84.97% <ø> (ø) |
This is a pretty meaty docstring. Thanks for the contribution @rowangayleschaefer and hopefully the build error gets fixed soon!
hopefully the build error gets fixed soon!
I merged the PR that fix this less than a minute ago!
Thank you! Should I make another PR to amend the notes that you made?
Thank you! Should I make another PR to amend the notes that you made?
Hi @rowangayleschaefer
You should be able to accept the changes that @OriolAbril suggested.
Then, you can pull those changes down locally with git pull origin rowan_5459
@cluhmann @OriolAbril Feel free to correct me if I am thinking of something else.
Done!
@rowangayleschaefer you should rebase your branch to make sure it contains all the changes from main
(and therefore includes the fix for the docs), otherwise we won't get the PR docs preview.
You can do that with the following commands:
git fetch origin # make sure local git is aware of latest changes in the fork
git fetch upstream # idem, but for main pymc repo
git checkout rowan_5459 # make sure you are on the right branch
git pull # get the changes from the fork (the updates from my suggestions)
git rebase upstream/main # get the changes from pymc main branch
git push --force-with-lease # push the changes to the PR
Should I make another PR to amend the notes that you made?
The answer to this is nearly always no. A pull request is a request to a project to merge the changes from a branch into another one (the two branches need to be of the same repo but don't need to be on the same fork). Therefore, in order to update a PR you should commit to that branch, then push. You can see that now, the PR includes 7 commits instead of the original 3 because my suggestions have been added as commits to this branch. After the git pull
step you will also see those commits on your local branch if you use git log
and if needed you can continue to work on the files locally and then commit and push in order to update the PR without needing to open a new one.
@rowangayleschaefer Let us know if you have any questions on this PR.
Thank you @OriolAbril @reshamas Didn't see this before. I just tried rebase + push with instructions.
Maybe the deflist inside the quotes looks better for the kwarg descriptions? Preview of both options: https://pymcio--6114.org.readthedocs.build/projects/docs/en/6114/api/generated/pymc.smc.sample_smc.html cc @rowangayleschaefer @ricardoV94 @aloctavodia
@rowangayleschaefer this looks great. Just a warning that I might try to merge #6171 first, in which case this PR will probably have to be updated
FYI. #6171 has been merged in. #6174 has been merged in.
@rowangayleschaefer Would you like to sync and then update the PR?
After rebasing I'll now merge, so all these nice docs fixes get rolled out with the upcoming v4.4.0
🚀
Thanks @rowangayleschaefer and reviewers!
Thank you Rowan