-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Start on API documentation #125
Conversation
Codecov Report
@@ Coverage Diff @@
## master #125 +/- ##
==========================================
- Coverage 94.3% 94.26% -0.05%
==========================================
Files 11 11
Lines 369 366 -3
==========================================
- Hits 348 345 -3
Misses 21 21
Continue to review full report at Codecov.
|
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.
Although it might seem counterintuitive at first, I'd actually rather that most type definitions/fields not be included in the documentation, instead opting towards a more API-based approach. For example, rather than telling the user externally that there are fields for storing the independent variables, dependent variables, parameters, etc., I'd rather document the accessor functions:
Similarly, rather than guaranteeing that a cached version of the Jacobian is available in the jac
field, I'd much rather just guarantee that calculate_jacobian
returns the right result. The user doesn't need to care about how the Jacobian is calculated, or even that it's cached - instead, all we have to promise is that calculate_jacobian
does the right thing.
This mindset is based on the recent explicit split between interface and implementation; within ModelingToolkit, we're making an effort to make no guarantees about the internal implementation, instead giving users ways of using the library without respect to implementation. This allows us to retain the same API while making our internals work differently.
Overall, though, I'm very excited about this PR! It'll be great to have documentation around ModelingToolkit.
@HarrisonGrodin makes sense, I've removed the documentation for all fields which seem to have accessors. Those were |
@HarrisonGrodin the most recent build passed but it didn't seem to update here. This should be ready to merge. |
It seems like there are still some TODOs? |
I actually intentionally left those in for other developers who know more about how the package works than I do. I can remove them if you don't want them in master. |
@HarrisonGrodin merge conflicts resolved, should I remove the TODOs or leave them in? |
The hold up is really that we don't know what the public API really should be yet... but screw it. Let's merge and keep refining it. |
Added basic docstrings to most of the core types and a few functions. Created documentation skeleton with API page showing docstrings.