Repository: spacetelescope/jwst

Issue:
Update set_velocity_aberration to use DataModel
_Issue [JP-2090](https://jira.stsci.edu/browse/JP-2090) was created on JIRA by [Jonathan Eisenhamer](https://jira.stsci.edu/secure/ViewProfile.jspa?name=eisenham):_

`set_velocity_aberration` uses `astropy.io.fits`. Update to use `jwst.datamodels`

As a subtask, explore other similar scripts and also update.

As a subtask, `set_telescope_pointing` has already been updated. However, it uses the specific `Level1bModel`. This should be changed to using `jwst.datamodels.open` to allow other models.

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) Report
Attention: `1 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`731ac25`)](https://app.codecov.io/gh/spacetelescope/jwst/commit/731ac25b347f792a41613f43c6e5b7ebde528bb6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) 75.14% compared to head [(`693c102`)](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) 75.14%.

| [Files](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Patch % | Lines |
|---|---|---|
| [jwst/lib/set\_velocity\_aberration.py](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-andzdC9saWIvc2V0X3ZlbG9jaXR5X2FiZXJyYXRpb24ucHk=) | 90.00% | [1 Missing :warning: ](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) |

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master    #8285   +/-   ##
=======================================
  Coverage   75.14%   75.14%           
=======================================
  Files         470      470           
  Lines       38615    38621    +6     
=======================================
+ Hits        29017    29023    +6     
  Misses       9598     9598           
```

| [Flag](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [nightly](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | `77.37% <ø> (-0.01%)` | :arrow_down: | Carriedforward from [731ac25](https://app.codecov.io/gh/spacetelescope/jwst/commit/731ac25b347f792a41613f43c6e5b7ebde528bb6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) |

*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/spacetelescope/jwst/pull/8285?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope).

hbushouse: Did you do a regression test run?  I'm not sure we have any tests that cover this script, but it can't hurt.
emolter: > Did you do a regression test run? I'm not sure we have any tests that cover this script, but it can't hurt.

I did not - the test coverage is inside lib/tests/test_velocity_aberration.py.  I will do a regression test run anyway, though
emolter: started [this Jenkins run](https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1265/)
hbushouse: Regression test run results are clean and JP-896 has been verified to no longer be an issue (fixed long ago in DMS build 7.8), so no further updates are needed to fix any old issues in this PR.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 8f94e2aae..97c2c3507 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -93,6 +93,12 @@ jump
 
 - Removed a unit test in Jump that was moved to STCAL to decrease
   the coupling of the two repos. [#8319]
+  
+lib
+---
+
+- Updated ``set_velocity_aberration`` to use datamodels instead of `astropy.io.fits` for opening
+  and manipulating input files. [#8285]
 
 outlier_detection
 -----------------
@@ -170,6 +176,9 @@ residual_fringe
 scripts
 -------
 
+- Updated ``set_velocity_aberration`` to have an optional ``--force-level1bmodel``
+  flag. [#8285]
+
 - Remove ``migrate_data`` and ``move_wcs`` scripts. [#8321]
 
 source_catalog
diff --git a/jwst/lib/set_velocity_aberration.py b/jwst/lib/set_velocity_aberration.py
index 9aed973c9..4e05f42ed 100644
--- a/jwst/lib/set_velocity_aberration.py
+++ b/jwst/lib/set_velocity_aberration.py
@@ -20,9 +20,10 @@ in the header other than what is required by the standard.
 
 import logging
 import numpy as np
-import astropy.io.fits as fits
 from gwcs.geometry import SphericalToCartesian, CartesianToSpherical
 from scipy.constants import speed_of_light
+import jwst.datamodels as dm
+from jwst.datamodels import Level1bModel
 
 # Configure logging
 logger = logging.getLogger(__name__)
@@ -114,29 +115,36 @@ def compute_va_effects(velocity_x, velocity_y, velocity_z, ra, dec):
     return scale_factor, apparent_ra, apparent_dec
 
 
-def add_dva(filename):
+def add_dva(filename, force_level1bmodel=True):
     """
     Given the name of a valid partially populated level 1b JWST file,
     determine the velocity aberration scale factor and apparent target position
     in the moving (telescope) frame.
 
     It presumes all the accessed keywords are present (see first block).
-    """
-    hdulist = fits.open(filename, mode='update')
-    pheader = hdulist[0].header
-    sheader = hdulist['SCI'].header
 
-    # compute the velocity aberration information
+    Parameters
+    ----------
+    filename : str
+        The name of the file to be updated.
+    force_level1bmodel : bool, optional.
+        If True, the input file will be force-opened as a Level1bModel.  If False,
+        the file will be opened using the generic DataModel.  The default is True.
+    """
+    if force_level1bmodel:
+        model = Level1bModel(filename)
+    else:
+        model = dm.open(filename)
     scale_factor, apparent_ra, apparent_dec = compute_va_effects(
-        velocity_x=sheader['JWST_DX'],
-        velocity_y=sheader['JWST_DY'],
-        velocity_z=sheader['JWST_DZ'],
-        ra=sheader['RA_REF'],
-        dec=sheader['DEC_REF']
+        velocity_x=model.meta.ephemeris.velocity_x_bary,
+        velocity_y=model.meta.ephemeris.velocity_y_bary,
+        velocity_z=model.meta.ephemeris.velocity_z_bary,
+        ra=model.meta.wcsinfo.ra_ref,
+        dec=model.meta.wcsinfo.dec_ref,
     )
 
     # update header
-    pheader['VA_RA'] = apparent_ra
-    pheader['VA_DEC'] = apparent_dec
-    sheader['VA_SCALE'] = scale_factor
-    hdulist.close()
+    model.meta.velocity_aberration.scale_factor = scale_factor
+    model.meta.velocity_aberration.va_ra_ref = apparent_ra
+    model.meta.velocity_aberration.va_dec_ref = apparent_dec
+    model.save(filename)
diff --git a/jwst/scripts/set_velocity_aberration.py b/jwst/scripts/set_velocity_aberration.py
index 0db01146e..8a0f0792b 100755
--- a/jwst/scripts/set_velocity_aberration.py
+++ b/jwst/scripts/set_velocity_aberration.py
@@ -22,6 +22,7 @@ in the header other than what is required by the standard.
 import logging
 import sys
 import warnings
+import argparse
 
 from jwst.lib.set_velocity_aberration import add_dva
 
@@ -30,10 +31,29 @@ logger = logging.getLogger(__name__)
 logger.addHandler(logging.NullHandler())
 
 
+def parse_args(args):
+    parser = argparse.ArgumentParser(
+        prog='set_velocity_aberration',
+        description='Add velocity aberration correction information to FITS files.',
+    )
+    parser.add_argument(
+        'filename',
+        nargs='+',
+        help='FITS file(s) to which to add the velocity aberration correction information.'
+    )
+    parser.add_argument(
+        '-f', '--force_level1bmodel',
+        choices=[1,0],
+        default=1,
+        type=int,
+        help='Force the input file to be treated as a Level1bModel. Options 0 (do not force) or 1 (yes, force). Default is 1.'
+    )
+    return parser.parse_args(args)
+
+
 def main():
-    if len(sys.argv) <= 1:
-        raise ValueError('missing filename argument(s)')
-    for filename in sys.argv[1:]:
+    args = parse_args(sys.argv[1:])
+    for filename in args.filename:
         add_dva(filename)
 
 


Test patch:
diff --git a/jwst/lib/tests/test_velocity_aberration.py b/jwst/lib/tests/test_velocity_aberration.py
index 6ab4b775c..8cbed1d3f 100644
--- a/jwst/lib/tests/test_velocity_aberration.py
+++ b/jwst/lib/tests/test_velocity_aberration.py
@@ -2,10 +2,9 @@
 Test script for set_velocity_aberration.py
 """
 import subprocess
-
 from numpy import isclose
 from astropy.io import fits
-
+import jwst.datamodels as dm
 from jwst.lib.set_velocity_aberration import compute_va_effects
 
 # Testing constants
@@ -33,14 +32,13 @@ def test_compute_va_effects_zero_velocity():
 def test_velocity_aberration_script(tmpdir):
     """Test the whole script on a FITS file"""
     path = str(tmpdir.join("velocity_aberration_tmpfile.fits"))
-    hdulist = fits.HDUList([fits.PrimaryHDU(), fits.ImageHDU(name='SCI')])
-    hdulist['SCI'].header['JWST_DX'] = GOOD_VELOCITY[0]
-    hdulist['SCI'].header['JWST_DY'] = GOOD_VELOCITY[1]
-    hdulist['SCI'].header['JWST_DZ'] = GOOD_VELOCITY[2]
-    hdulist['SCI'].header['RA_REF'] = GOOD_POS[0]
-    hdulist['SCI'].header['DEC_REF'] = GOOD_POS[1]
-    hdulist.writeto(path)
-    hdulist.close()
+    model = dm.ImageModel()
+    model.meta.ephemeris.velocity_x_bary = GOOD_VELOCITY[0]
+    model.meta.ephemeris.velocity_y_bary = GOOD_VELOCITY[1]
+    model.meta.ephemeris.velocity_z_bary = GOOD_VELOCITY[2]
+    model.meta.wcsinfo.ra_ref = GOOD_POS[0]
+    model.meta.wcsinfo.dec_ref = GOOD_POS[1]
+    model.save(path)
 
     subprocess.check_call(["set_velocity_aberration", path])
 
diff --git a/jwst/scripts/tests/test_argparsers.py b/jwst/scripts/tests/test_argparsers.py
new file mode 100644
index 000000000..d392855d7
--- /dev/null
+++ b/jwst/scripts/tests/test_argparsers.py
@@ -0,0 +1,25 @@
+import pytest
+from jwst.scripts.set_velocity_aberration import parse_args
+
+
+def test_argparse_set_velocity_aberration():
+
+    args = parse_args(['--force_level1bmodel', '0', 'filename0', 'filename1', 'filename2'])
+    assert args.filename == ['filename0', 'filename1', 'filename2']
+    assert isinstance(args.force_level1bmodel, int)
+    assert not bool(args.force_level1bmodel)
+
+    args2 = parse_args(['filename0', 'filename1', 'filename2', '--force_level1bmodel', '1'])
+    assert args.filename == args2.filename
+
+    args3 = parse_args(['filename0', 'filename1', 'filename2'])
+    assert args3.force_level1bmodel is True
+
+
+def test_argparse_set_velocity_aberration_bad_input(capsys):
+
+    with pytest.raises(SystemExit):
+        parse_args(['--force_level1bmodel', '1'])
+
+    _,err = capsys.readouterr()
+    assert err[:6] == "usage:"