WIP: JP-3563: Raise warning instead of error for unexpected values in config

[melanieclarke]

Resolves JP-3563

Closes spacetelescope/jwst#8335

For future-proofing pipeline runs with new steps, unexpected values in pipeline or step configuration now raise warnings instead of ValidationErrors. Unexpected steps or parameter values are ignored. This allows an older pipeline version to be used with a newer parameter reference file.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.14%. Comparing base (531f752) to head (2d0e8e0).
Report is 76 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   69.46%   71.14%   +1.68%     
==========================================
  Files          24       24              
  Lines        1634     1653      +19     
==========================================
+ Hits         1135     1176      +41     
+ Misses        499      477      -22     

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

[ddavis-stsci]

LGTM

[braingram]

I'm a little surprised that test_extra_parameter isn't failing in the jwst downstream job.
https://github.com/spacetelescope/jwst/blob/f8665574b9e8ad314c003f3661ca50e515719cad/jwst/stpipe/tests/test_step.py#L563-L565
A quick glance makes me think that test is checking that the step should raise an exception for an unexpected value. Is that still passing (and raising an exception) because an unknown parameter on the command line is expected to produce an error whereas a parameter in a file should now produce a warning?

[melanieclarke]

A quick glance makes me think that test is checking that the step should raise an exception for an unexpected value. Is that still passing (and raising an exception) because an unknown parameter on the command line is expected to produce an error whereas a parameter in a file should now produce a warning?

Yes, the current change is specifically for parsing configuration files - directly passed parameters on the command line or in step arguments will still raise a ValidationError. I think this is as requested, to support parameter reference files in CRDS that are a little ahead of the pipeline code.

[melanieclarke]

@nden - I don't seem to have permissions on stpipe to request specific reviewers, but I think this one is ready for review, as discussed.

[braingram]

The code changes look good to me. I left a comment on JP-3563 about my concerns for turning this error into a warning.

@melanieclarke would you run the jwst and romancal regtests with this PR?

[melanieclarke]

@braingram - I can run the jwst regtests, but I don't have access to the romancal ones.

jwst tests started here:
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1495/

[braingram]

Thanks! I started a romancal run here:
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/807/

EDIT: passed with no errors

[melanieclarke]

There were a few failures in the jwst regtests, but they're unrelated to this PR.

[melanieclarke]

Setting this back to draft to explore warning for extra parameters only when pulling from CRDS.

[melanieclarke]

Added some draft changes that would warn for extra values in configuration files retrieved from CRDS only.

Note that this adds a validation step earlier in the process than it currently happens, so will add default values to the configuration where they did not exist before. We'd need to check that this doesn't have downstream effects (i.e. default values overwriting real ones).

Also, I have not added any optional controls for this process yet: it always strips extra values from CRDS when retrieved via get_config_from_reference. It's not clear to me at what point in the process that might need to be turned off.

In its current form, there's one downstream unit test in jwst that will need to be updated.

I'm leaving this PR at draft status: it needs more discussion.

[melanieclarke]

Closing: the associated JP ticket is withdrawn.