Updated docstrings in pymc.step_methods.metropolis for BinaryGibbsMetropolis to follow numpydoc #7212
Conversation
|
|
There was a problem hiding this comment.
As you can see in the preview: https://pymcio--7212.org.readthedocs.build/projects/docs/en/7212/api/generated/classmethods/pymc.step_methods.Metropolis.__init__.html, this is already a great improvement. I have added some extra comments to try and get rid of the pink text in the type info and have everything as links.
The only general comment is to add the , optional or default xyz info too when relevant as covered in https://pymc-data-umbrella.xyz/en/latest/contributing/tutorials/docstring_tutorial.html#parameters
pymc/step_methods/metropolis.py
Outdated
| List of value variables for sampler | ||
| S: standard deviation or covariance matrix | ||
| S : standard deviation or covariance matrix |
There was a problem hiding this comment.
standard deviation or covariance matrix aren't types, this is mixing type with description. I think I have never used Metropolis, so my guess about behaviour here might be wrong but here is a proposal:
S : array_like with shape (N,) or (N, N), optional
Then in proposal_dist description:
Function that returns zero-mean deviates when parameterized with
`S` (and n). If `S` is one dimensional, it defaults to a normal, with `S` as its standard deviation;
If `S` is two dimensional, it defaults to a multivariate normal with `S` as its covariance matrix.
There was a problem hiding this comment.
I have changed this one based on your suggestions. I also changed S description to match.
pymc/step_methods/metropolis.py
Outdated
| Function that returns zero-mean deviates when parameterized with | ||
| S (and n). Defaults to normal. | ||
| scaling: scalar or array | ||
| scaling : scalar or array |
There was a problem hiding this comment.
| scaling : scalar or array | |
| scaling : scalar or array_like, default 1 |
for inputs unless it is required they are already a numpy array it is better to use array_like instead.
There was a problem hiding this comment.
I have changed this one based on your suggestions.
pymc/step_methods/metropolis.py
Outdated
| The frequency of tuning. Defaults to 100 iterations. | ||
| model: PyMC Model | ||
| model : PyMC Model |
There was a problem hiding this comment.
| model : PyMC Model | |
| model : Model |
using only Model should trigger the use of this alias: https://github.com/pymc-devs/pymc/blob/main/docs/source/conf.py#L73 and render that as a link to the pymc.Model api documentation.
note, it might not work right now because a while back some objects were undocumented. For example, the Model class might not be currently documented at pymc.Model. If that were the case ignore it; the docs for pymc.Model will be fixed at some point and everything will work again.
There was a problem hiding this comment.
I have changed this one based on your suggestions.
| Optional model for sampling step. Defaults to None (taken from context). | ||
| mode: string or `Mode` instance. | ||
| mode : string or `Mode` instance. |
There was a problem hiding this comment.
I don't really know what Mode is so it would be nice to have it as a link, unfortunately I can't give a suggestion unless I know where it should point to. Hopefully someone else can comment on this
There was a problem hiding this comment.
It's a pytensor thing... Not sure we have a more precise signature anywhere. The class is https://github.com/pymc-devs/pytensor/blob/a76172ee6a9753266150fc7bac4dd66906967a26/pytensor/compile/mode.py#L275
But it can also be one of a predefined set of strings :D
Description
First time contributor to open-source and the project for me.
I updated the docstring of BinaryGibbsMetropolis to follow numpydoc format. I also updated the Metropolis init while trying to check how Sphinx works (deleting it would be pointless so I keep it there).
Related Issue
Checklist
Type of change
📚 Documentation preview 📚: https://pymc--7212.org.readthedocs.build/en/7212/