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 30015: Update conf.py for documenteer 0.6 #112

Merged
merged 1 commit into from Dec 7, 2021
Merged

Conversation

jonathansick
Copy link
Member

Update conf.py for the new documenteer 0.6 configuration API. Based on
the latest updates in the package template
(https://github.com/lsst/templates/tree/master/project_templates/stack_package)

@timj
Copy link
Member

timj commented Dec 7, 2021

@jonathansick do you want me to review some of these? They don't all have to be reviewed and merged on the same day. Incrementally works fine.

@jonathansick
Copy link
Member Author

If you'd like to review (and even merge) that'd be great. Thanks.

Update conf.py for the new documenteer 0.6 configuration API.  Based on
the latest updates in the package template
(https://github.com/lsst/templates/tree/master/project_templates/stack_package)
"""

from documenteer.sphinxconfig.stackconf import build_package_configs
import lsst.ap.association
from documenteer.conf.pipelinespkg import *
Copy link
Member

Choose a reason for hiding this comment

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

As a general comment (too late now given it's everywhere) it would seem less like magic and more PEP8 as:

import documenteer.conf.pipelinespkg as p

p.project = "ap_association"

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you check this with a Sphinx run? I'm not sure this would be compatible with the namespacing that Sphinx expects for configurations.

Copy link
Member

Choose a reason for hiding this comment

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

Aha. I'll take a look. I'm sure you are right -- namespace shenanigans seems right.

Copy link
Member

Choose a reason for hiding this comment

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

Right. Falls over completely. I'll merge this.

@timj timj merged commit b991328 into main Dec 7, 2021
@timj timj deleted the tickets/DM-30015 branch December 7, 2021 22:42
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