-
Notifications
You must be signed in to change notification settings - Fork 4
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
Docstrings for all epimodel types and generate_latent_infs plus named arguments for constructors #138
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
=======================================
Coverage 97.61% 97.61%
=======================================
Files 13 13
Lines 168 168
=======================================
Hits 164 164
Misses 4 4 ☔ View full report in Codecov by Sentry. |
I've done everything in the opening description except for the example usage of the EpiModels and The reason for this is that the pattern of doc testing against an expected result didn't seem to meet what we want from the examples and might lead into complexity around different random seed implementation on different OS etc. An alternative approach which just checks the examples run didn't seem to do anything that our testing CI doesn't do anyway. Happy to get pushed back on this. |
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.
This is nice. It doesn't look like you have implemented field or return information for functions or made use of DocStringExtensions
. I think we need at least a clear return field but perhaps in another issue.
This is a draft PR for on going work on documenting
AbstractEpiModel
subtypes and methods that dispatch on them.Current focus and decisions:
generate_latent_infs
dispatch on genericAbstractEpiModel
.DirectInfections
goes beyond just constructing the struct, but also its main usage withgenerate_latent_infs
.testitem
s. However, a next step could be to combine as per discussed in Doctests with @testitem #48. I've readvised this... simple doctests will be included.This is aimed at addressing #137 which is part of #35 and #97 . It sets my current expectation of docstrings #135 and #126 .
Extra activity
@kwdef
I am doing it, because this also makes example usage easier. This moves towards addressing Make all constructors have named and not positional arguments. #133 , but won't close this issue.