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-15872: Incorporate AP documentation into pipelines.lsst.io #48
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 looks really solid. I haven't rendered this locally yet, but it seems to hit all the marks.
Just a few bits of feedback that you could consider but aren't necessary at this point:
- For the command
ap_verify.py
, I think it'd be good to always writeap_verify.py
to disambiguate from the package name and because that's the command that's actually run. (now why LSST puts extensions on its scripts is another matter for discussion :) ) - Along the same lines, you might want to look at using the
program
domain. This will let you "namespace" the option directives so that they don't conflict with documentation of similarly-named options in other scripts out there.
|
||
In order to be supported by ``ap_verify``, datasets must be registered in ``ap_verify``'s :ref:`configuration file<ap-verify-configuration-dataset>` and registered as an *optional* EUPS dependency of ``ap_verify``. | ||
In order to be supported by ``ap_verify``, datasets must be registered in ``ap_verify``'s :ref:`configuration file<ap-verify-configuration-dataset>`. |
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.
Just FYI, possessives of inline syntax often need to be escaped, like this:
``ap_verify``\ ’s
I haven't compiled this yet (long story) so I can't say if the rendering is mangled in this case or not.
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 compiles fine. I think it's the single backticks that you need to watch out for.
I'm already using |
Dataset pages cannot yet be included in pipelines.lsst.io; link to GitHub repositories instead.
ea8e3a3
to
7702656
Compare
This PR updates the documentation to current data package conventions (in particular, merging the package and module home pages, and transforming the
.measurements
subpackage documentation into a contributing guide) and fixes build errors detected when building withpipelines_lsst_io
.