-
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
Partial documentation of EpiAware
and also sets documentation standard
#85
Conversation
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 looking really nice and from a skim all looks good. Any idea what is going on in CI?
The answer is that I was not being careful in adding deps!
Will fix in the morning! |
sorry I haven't yet got to this in detail and also for the mess that #89 will bring to this |
shall i look at this now? Do you want to ping me to review (top right when ready?) |
Yeah, there is not alot to review:
Second one I only just noticed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
=======================================
Coverage 97.74% 97.74%
=======================================
Files 6 6
Lines 133 133
=======================================
Hits 130 130
Misses 3 3 ☔ View full report in Codecov by Sentry. |
This seems like a cleaner solution than escaping to me for Latex but we could address in its own issue. I definitely like |
I did a quick try on that, and the problem was that E.g. |
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.
It looks like the docs are out of sync with the recent name changes and that AbstractModel
isn't present (despite it appearing that this branch is up to date.
ah okay. Lets port to another issue for discussion. |
It looks like this was missed in #89 but perhaps we are fine with it not being exported? |
AbstractModel doc mention Co-authored-by: Sam Abbott <[email protected]>
Co-authored-by: Sam Abbott <[email protected]>
Co-authored-by: Sam Abbott <[email protected]>
Co-authored-by: Sam Abbott <[email protected]>
Co-authored-by: Sam Abbott <[email protected]>
Co-authored-by: Sam Abbott <[email protected]>
Co-authored-by: Sam Abbott <[email protected]>
I think this is ok now |
Aweesome will review now. Could you resolve out the things that are sorted? |
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.
A few minor issues but otherwise good to go.
Co-authored-by: Sam Abbott <[email protected]>
This PR gives partial documentation to
EpiAware
, mainly the module.Also gives a standard for using
DocStringExtensions
.