JP-4036: Add quad observables to AMI outputs

[rcooper295]

Resolves JP-4036

This PR addresses JP-4036, which adds four-hole observables (closure amplitudes and quad phases) to the AMI3 oi.fits products of the AmiAvergage and AmiNormalize steps, in a new extension called OI_Q4.

This PR depends on spacetelescope/stdatamodels#582

Tasks

• If you have a specific reviewer in mind, tag them.
• add a build milestone, i.e. Build 12.0 (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 changelog readme for instructions)
  • 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

[emolter]

regtests with the stdatamodels PR branch: https://github.com/spacetelescope/RegressionTests/actions/runs/17587518587

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.84%. Comparing base (9915794) to head (e1267b8).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9812      +/-   ##
==========================================
- Coverage   83.03%   82.84%   -0.20%     
==========================================
  Files         366      366              
  Lines       37650    37536     -114     
==========================================
- Hits        31261    31095     -166     
- Misses       6389     6441      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[emolter]

@rcooper295 Thanks very much for this PR! As I said on the stdatamodels one, I'll test the combination of the two PRs locally. Would you please:

• Add a changelog fragment. It explains how to do this in the PR template at the top of the file, but basically you need to make a new .rst file named changes/9812.ami.rst with a line in there saying that closure amplitudes and quad phases are now being computed in those steps.
• Have a look at, and address, the failing unit tests. Probably you need to add q4 quantities to some mock data being created by the step.
• Address code/documentation style issues raised by the pre-commit job; see here. There are instructions here for how to set up your development environment so that the pre-commit job runs locally for you every time you make a commit. This is the recommended workflow, but if you'd rather just fix them manually that's totally fine too.

Please do let me know if you are having trouble with any of these steps, and I'd be happy to help. Also, if you just simply don't have time/interest in doing this yourself, I'd be happy to take over from here - I know making pipeline code look nice and well-tested is probably slightly "above and beyond" your job description.

[emolter]

I'm setting the build to 12.1 since we have 8 1/2 more workdays until the deadline, and the changes are not that huge, but with the understanding that this may not make it in.

[rcooper295]

Thanks so much Ned! I'll try to finish that up this afternoon so you have as much time as possible to review/merge.

[emolter]

Thanks Rachel, the code looks good to me overall. I made a few suggestions.

I tested running the pipeline with this branch and the stdatamodels branch. It seems to work, producing fits files that include a Q4 extension containing real data values. These models are loadable into stdatamodels.

One unrelated thing I noticed is that it seems the TIME column of the table is all zero-valued. The reason I say it's unrelated is because this appears to also be true for the other tables (e.g. OI_T3) too. Is that expected? What is that column supposed to do?

[rcooper295]

One unrelated thing I noticed is that it seems the TIME column of the table is all zero-valued. The reason I say it's unrelated is because this appears to also be true for the other tables (e.g. OI_T3) too. Is that expected? What is that column supposed to do?

I believe it's just what's expected; from the oifits v2 standard paper: "In OIFITS version 2 only MJD and DATE-OBS
must be used to express time. The TIME column is retained only for backwards compatibility and must contain zeros ."

I think I finally made my way through the formatting errors although it still says the pre-commit.ci check failed, not sure how to proceed.

[emolter]

It looks like the formatting errors are caused by the ruff-format job, which are just auto-fixes to things like the number of spaces, single vs double quotes, characters in a line, etc. Locally, if you have pre-commit installed, you should be able to just do pre-commit run ruff-format. Or if you installed our dev environment via these instructions, it should auto-fix these when you attempt to make a commit (although you'll need to run git add and git commit twice)

[emolter]

Thanks for updating this with all my previous suggestions! This is looking close to ready now. I left a few small suggestions about the unit tests.

Note the tests are expected to fail in our CI job because it's not pointing to the proper stdatamodels branch. But testing them locally they look good (except the one typo I flagged).

[emolter]

restarting regtests https://github.com/spacetelescope/RegressionTests/actions/runs/17587518587

[emolter]

This looks good to me now. We will need to wait on merging this until the accompanying stdatamodels PR is ready and merged.

[melanieclarke]

Looks good, just one question about the INT_TIME column.

[melanieclarke]

LGTM, pending merge of the stdatamodels PR. I'll leave it to @emolter to manage the merge and okify.

[emolter]

new round of regtests for okify https://github.com/spacetelescope/RegressionTests/actions/runs/17653021304