Improve NamedDist docstring with fields documentation#1370
Conversation
|
@penelopeysm This is my first contribution to DynamicPPL.jl. |
penelopeysm
left a comment
penelopeysm
left a comment
There was a problem hiding this comment.
Thank you! I recognise this is quite a small PR and it might seem nitpicky of me, but I'd still like to point out a couple of things to perhaps improve it further. None of these are intended as criticisms at all.
| using FillArrays: Fill | ||
|
|
||
| """ | ||
| NamedDist{variate, support, Td, Tv} |
There was a problem hiding this comment.
This line is totally correct -- however, because it is a description of the type, it doesn't tell people how to construct one of them. I personally like docs when they tell me about the constructors themselves (and I find that the type itself is not that important -- you only need it if you are writing a function that dispatches on the type, which is much less likely for the average user user).
| NamedDist{variate, support, Td, Tv} | |
| NamedDist(dist::Distribution, name::Symbol) |
| # Fields | ||
|
|
||
| - `dist::Td`: The underlying distribution. | ||
| - `name::Tv`: The name of the random variable as a `VarName`. | ||
| """ | ||
| struct NamedDist{variate,support,Td<:Distribution{variate,support},Tv<:VarName} <: | ||
| Distribution{variate,support} | ||
| dist::Td | ||
| name::Tv | ||
| end |
There was a problem hiding this comment.
You can actually add strings on top of the fields themselves, and then use $(TYPEDFIELDS) in the docstring which will automatically pull in the comments. (The TYPEDFIELDS identifier is provided by DocStringExtensions.jl: https://docstringextensions.juliadocs.org/latest/)
This helps ensure that the docstring stays in sync with the source code. For an example of this you can see e.g. LogPriorAccumulator in src/accumulators/default.jl (or equivalently grep for TYPEDFIELDS in the repo, there are several other examples).
|
@penelopeysm Thank you for the feedback. I have updated the docstring to use the constructor signature and |
Thank you for the review and for merging. Looking forward to contributing more. |
2 participants
What does this PR do?
The NamedDist struct previously had a very brief one line description which lacked detailed documentation.
Changes made
Added a full struct signature to the docstring header.
Included a # Fields section to explain the dist and name fields.
Updated the documentation style to match the standard Julia documentation format used across the rest of the codebase.
Why?
Providing proper documentation for the fields helps both users and developers understand the purpose of the struct without needing to dive into the source code itself.