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

RCAL-911 & 932: remove units from MOS and ELP pipelines. #1445

Conversation

mairanteodoro
Copy link
Collaborator

@mairanteodoro mairanteodoro commented Oct 8, 2024

Resolves RCAL-911
Resolves RCAL-932

This PR modifies several files so that units are not used anymore. Additionally, the filename used in the regression tests was changed to remove the ggsaa component.

The information about units is still retained in the RAD schema.

Tasks

  • request a review from someone specific, to avoid making the maintainers review every PR
  • add a build milestone, i.e. 24Q4_B15 (use the latest build if not sure)
  • Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • update or add relevant tests
    • update relevant docstrings and / or docs/ page
    • start a regression test and include a link to the running job (click here for instructions)
      • Do truth files need to be updated ("okified")?
        • after the reviewer has approved these changes, run okify_regtests to update the truth files
  • if a JIRA ticket exists, make sure it is resolved properly
news fragment change types...
  • changes/1445.general.rst: Remove units from romancal.

mairanteodoro and others added 30 commits October 7, 2024 11:48
… ELP (spacetelescope#1373)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Zach Burnett <[email protected]>
Co-authored-by: Brett <[email protected]>
Copy link
Collaborator

@schlafly schlafly left a comment

Choose a reason for hiding this comment

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

Thanks Mairan. This looks great. I left one minor comment about deleting some presently commented-out code. Please spot check one of the L2 and L3 pipeline outputs for unexpected differences but this looks good to me.

FYI, @ddavis-stsci , this contains two sets of changes that touch all of the regtest files:

  • changing the regtest filenames to remove the ggsaa component
  • removing all of the units.

@mairanteodoro mairanteodoro added this to the 25Q1_B16 milestone Oct 16, 2024
@mairanteodoro mairanteodoro changed the title RCAL-932: remove units in exposure level pipeline RCAL-932: remove units from romancal. Oct 16, 2024
@mairanteodoro mairanteodoro changed the title RCAL-932: remove units from romancal. RCAL-911 & 932: remove units from MOS and ELP pipelines. Oct 16, 2024
@mairanteodoro mairanteodoro marked this pull request as ready for review October 16, 2024 17:22
@mairanteodoro mairanteodoro requested a review from a team as a code owner October 16, 2024 17:22
@mairanteodoro mairanteodoro mentioned this pull request Oct 16, 2024
10 tasks
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 16, 2024
@PaulHuwe PaulHuwe merged commit f23b480 into spacetelescope:main Oct 17, 2024
30 of 31 checks passed
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.

6 participants