Skip to content
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

DM-15162: Improve documentation for DataIdContainer #57

Merged
merged 2 commits into from Jul 21, 2018
Merged

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented Jul 20, 2018

and also clean up some other docs.

Copy link
Contributor

@parejkoj parejkoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about where to document.

Otherwise, it would have been much easier to review if the 80-char reflow was on a separate commit from the wording changes.

the override class as ``ContainerClass`` for ``add_id_argument``. (If you don't want the argument parser
to compute data references, you may use this class and specify ``doMakeDataRefList=False`` in
``add_id_argument``.)
**Fields:**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be documented in a Parameters block, or should they be declared as class members and documented there?

https://developer.lsst.io/python/numpydoc.html#examples-of-constant-and-class-attribute-docstrings

especially for `DataIdContainer` and the `level` argument
@r-owen
Copy link
Contributor Author

r-owen commented Jul 21, 2018

I moved the field documentation and rebased the commit as per your suggestions

@r-owen r-owen merged commit 44ca919 into master Jul 21, 2018
@ktlim ktlim deleted the tickets/DM-15162 branch August 25, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants