-
Notifications
You must be signed in to change notification settings - Fork 28
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
TweakReg: doc updates. #673
TweakReg: doc updates. #673
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #673 +/- ##
==========================================
+ Coverage 63.74% 64.56% +0.81%
==========================================
Files 78 78
Lines 4115 4117 +2
==========================================
+ Hits 2623 2658 +35
+ Misses 1492 1459 -33
*This pull request uses carry forward flags. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
Looks good. Two minor questions about tweakreg features that make sense at L3 time but which I don't really understand at L2 time. One other thing that would be nice would be an example invocation of the pipeline with a custom catalog. There's a lot of code documentation there but not very much user-facing documentation there.
55a717c
to
04c3f3f
Compare
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.
These docs still look good to me. I am a bit confused, though---mostly in my last review I wanted some clarification about what a couple things did when tweakreg is used at L2 time, which is the default case for Roman. There we don't have overlaps and some steps presumably get skipped. I think we should make this default case very clear.
I also thought that some user facing documentation of the custom catalog option would be valuable, since a lot of the docs talked about the API but as a user I would have liked to see e.g. an invocation that used that feature. I don't think that additional documentation is strictly necessary if it's hard or if we don't have a CLI to that bit of the step, though.
04c3f3f
to
18546a1
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
We added an "Examples" page to the |
Resolves RCAL-527
This PR addresses updates to the RomanCal version of TweakReg.
Checklist
CHANGES.rst
under the corresponding subsection