-
Notifications
You must be signed in to change notification settings - Fork 36
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
Documentation update #175
Documentation update #175
Conversation
Changed import order to be PEP8 compliant
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.
Other comments relate to content of new_instrument.rst:
- Supported Data Templates should include Madrigal
- A section with a list of the optional, but standard inst routines should be added.
- Where and how to add data acknowledgements should be added.
Added. Madrigal currently does not use Writing up documentation for current methods. |
Updates completed. We should have a larger conversation about how the acknowledgements are implemented. |
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.
I noted an area that needs tweaking, but otherwise looks good.
I was a bit surprised myself that the madrigal code didn't use functools. I don't remember why that is. Updating it would improve style consistency but wouldn't change overall functionality.
Updates to madrigal are out of scope here. Adding a note to #106. |
Closes #69, #94, and #153.
Summary of changes