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

Added several more images and updates for build context. #1082

Merged
merged 5 commits into from
Oct 7, 2024

Conversation

hbrown-ST
Copy link
Collaborator

@hbrown-ST hbrown-ST commented Oct 7, 2024

Resolves CCD-1508

More Doc updates for Data Release work to User Guide

news fragment change types...
  • changes/<PR#>.hst.rst: HST reference files
  • changes/<PR#>.jwst.rst: JWST reference files
  • changes/<PR#>.roman.rst: Roman reference files
  • changes/<PR#>.doc.rst: documentation change
  • changes/<PR#>.testing.rst: change to tests or test automation
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change

@@ -211,7 +211,7 @@ New Context

crds.bestrefs always computes best references with respect to a context which
can be explicitly specified with the `--new-context` parameter. If `--new-context`
is not specified, the default latest context is determined by consulting
is not specified, the default build context is determined by consulting
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is only the case for JWST. This needs to be clarified with something like: "{...} is not specified, the way the context is determined depends on the observatory. For Roman and HST, the default latest context is determined by consulting the CRDS server or looking in the local cache. For JWST, the default build context is determined by consulting the CRDS server to find the appropriate context for the calibration software version you have locally installed."

@@ -181,15 +181,15 @@ crds.get_default_context()
..........................

`get_default_context()` returns the name of the pipeline mapping which is
currently the Latest Context
currently the Build Context
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as above. get_default_context only defaults to build if the observatory is JWST. Otherwise it defaults to latest.

new files are being added but need additional testing in OPS. Deriving
from the *Latest Context* is a crude kind of reversion since CRDS
effectively branches around any existing subsequent contexts. use.
*Build Context* is the .pmap which is nominally in use by the pipeline.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, see above. I think it would best to add "build context" info, not replace latest with it. You could add to the first sentence: "Latest Context is the .pmap which is nominally in use by the Roman or HST pipeline. For JWST, see "build context" below.

Copy link
Collaborator

@alphasentaurii alphasentaurii left a comment

Choose a reason for hiding this comment

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

Need to clarify that JWST defaults to build context, while Roman and HST default to latest. Both context types are available to all missions, so I don't agree with replacing "latest" with "build" across the board here. We need to include information describing both, and where they apply.

Copy link
Collaborator

@alphasentaurii alphasentaurii left a comment

Choose a reason for hiding this comment

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

Great - these changes look good to me as far as I can tell. Let's at least start with this and we can make additional adjustments if needed later on.

@hbrown-ST hbrown-ST merged commit 423b4d3 into master Oct 7, 2024
4 of 10 checks passed
@hbrown-ST hbrown-ST deleted the DR-user-guide-update-2 branch October 7, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants