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.

Sign up for GitHub

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

Jump to bottom

DM-13878: Confirm that ap_verify's documentation is sphinx-buildable #27

Merged
merged 3 commits into from Apr 11, 2018
Merged

DM-13878: Confirm that ap_verify's documentation is sphinx-buildable #27

merged 3 commits into from Apr 11, 2018

Conversation

kfindeisen
Copy link
Member

This PR brings ap_verify in sync with other Sphinx-documented packages. Fortunately, most of the changes that needed to be made were cosmetic.

Since the biggest change by line count was converting three files from Windows to Unix line endings, this PR may be best reviewed commit by commit.

@kfindeisen
Force unix line endings.
be4f383
@kfindeisen
Standardize Sphinx configuration files.
0a3b364
@kfindeisen
Test integrated pipeline build.
143fbf5 
One broken link to the existing pipe_base docs has been fixed. Note
that ap_verify cannot be added to pipelines_lsst_io until it's part
of the Stack.
jonathansick
jonathansick approved these changes Apr 11, 2018
View reviewed changes
Copy link
Member

@jonathansick jonathansick left a comment

Choose a reason for hiding this comment

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

Just a heads up that you may not really need an extra module documentation directory for lsst.ap.verify.measurements. The intent for having a list of module directories in manifest.yaml was for edge-case packages like afw where the subpackages of lsst.afw are quite unrelated and it's really the subpackages that I wanted to highlight from the homepage. It wasn't really intended to separate lsst.packageA and lsst.packageA.subpackage in the homepage listing.

In this case, I think everything will work quite well if there's only the lsst.ap.verify directory. You'd just have to move the .. automodapi:: lsst.ap.verify.measurements directive to doc/lsst.ap.verify/index.rst.

@kfindeisen
Copy link
Member Author

kfindeisen commented Apr 11, 2018 •
edited

The intent of documenting lsst.ap.verify.measurements separately was that people might end up making lots of changes to that package in order to support new metrics. When we were developing demo metrics for ap_verify I found that I had to explain the reasoning behind the package to keep people from re-coupling measurements to the main ap_verify module. The automodapi call is mostly a formality, since those functions were not intended to be called from anywhere outside ap_verify.

@jonathansick
Copy link
Member

Ah, got it.

@kfindeisen kfindeisen merged commit 143fbf5 into master Apr 11, 2018
@kfindeisen kfindeisen deleted the tickets/DM-13878 branch November 30, 2018 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathansick jonathansick jonathansick approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@kfindeisen @jonathansick