Add helper functions to aid in migration of AsdfInFits from ASDF

[braingram]

This PR adds an asdf_in_fits submodule with 2 functions:

• open
• write

These are intended as near replacements for:

• opening an AsdfInFits file
• writing an AsdfInFits file

The signatures of the replacement functions were determined by surveying AsdfInFits usage outside of stdatamodels/jwst. These uses include:

• hcipy
  • read: call to asdf.fits_embed.AsdfInFits.open(filename)
  • write: call to ff = asdf.fits_embed.AsdfInFits(hdulist, tree); ff.write_to(filename, …)
• specutils
  • read: call to with asdf.open(filename) as af
  • read: call to with asdf.open(filename) as af
  • write: call to ff = asdf.fits_embed.AsdfInFits(hdulist, tree); ff.write_to(filename, …)
• jdaviz
  • read: call to with AsdfInFits.open as af
  • write: call to ff = asdf.fits_embed.AsdfInFits(hdulist, tree); ff.write_to(filename, …)

To summarize, the above uses include opening as a with context or as a function call and writing by creating an AsdfInFits object then calling write_to.

For reads using a with statement, calls like the following:

with asdf.open(filename) as af:  # or AsdfInFits.open

can be replaced with

with stdatmodels.asdf_in_fits.open(filename) as af:

The same function can be used outside of a with statement to allow replacing:

af = asdf.open(filename)

with

af = stdatamodels.asdf_in_fits.open(filename)

Finally writes like the following:

hdulist = fits.HDUList()
ff = asdf.fits_embed.AsdfInFits(hdulist, tree)
ff.write_to(filename, …)

can be replaced with

stdatamodels.asdf_in_fits.write(filename, tree)

This and several related forms (including passing in an empty or partially full hdu) is included as tests.

The rendering of the documentation appears odd as the numpy style docstrings don't appear to correctly render locally (there is no space between parameter names and types).

Checklist

• added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
• updated relevant tests
• updated relevant documentation
• updated relevant milestone(s)
• added relevant label(s)

[codecov]

Codecov Report

Base: 49.96% // Head: 50.31% // Increases project coverage by +0.34% 🎉

Coverage data is based on head (ea5db57) compared to base (9639bca).
Patch coverage: 76.76% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
+ Coverage   49.96%   50.31%   +0.34%     
==========================================
  Files         121      123       +2     
  Lines        7669     7765      +96     
==========================================
+ Hits         3832     3907      +75     
- Misses       3837     3858      +21     

Flag	Coverage Δ
unit	50.31% <76.76%> (+0.34%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/stdatamodels/asdf_in_fits.py	0.00% <0.00%> (ø)
src/stdatamodels/fits_support.py	17.34% <33.33%> (+0.10%)	⬆️
tests/test_asdf_in_fits.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[WilliamJamieson]

This should have an actual docs page. Indeed, fits_support in general should have a detailed documentation page anyways, so that we can link to it from the ASDF documentation.

[braingram]

JWST regression tests passed:
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST/2204/

[braingram]

This should have an actual docs page. Indeed, fits_support in general should have a detailed documentation page anyways, so that we can link to it from the ASDF documentation.

Are the docs for this package published anywhere?

Adding documentation to fits_support seems outside the scope of this PR.

What is your opinion on adding the migration documentation to ASDF as it has more thorough documentation and these functions are being added to aid in migration for ASDF users?

[WilliamJamieson]

Are the docs for this package published anywhere?

It should @zacharyburnett or @nden should know where it is (or we should create a RTD). There is a docs in this repo which is functional, though it does not contain much.

Adding documentation to fits_support seems outside the scope of this PR.

It seems entirely within the scope of this PR to me (in hindsight the documentation really should have been part of #110). Since ASDF-in-Fits support of some fashion is being moved specifically to this package meaning it needs to be documented. This PR is intended to add things to help support the transition of ASDF-in-Fits from asdf to stdatamodels. Documentation of this support is entirely in-scope of these changes IMOP (if you disagree, it still needs to happen and should be a follow-on PR).

What is your opinion on adding the migration documentation to ASDF as it has more thorough documentation and these functions are being added to aid in migration for ASDF users?

No. the documentation of something, fits_support and/or the "helper functions" for the ASDF-in-Fits transition should live directly alongside the code implementing this. Thus the migration documentation should live here. Insofar as the ASDF documentation, we will need to declare that ASDF-in-Fits is deprecated in the documentation, and then link to the migration guide here using an intersphinx link.

[braingram]

It seems entirely within the scope of this PR to me (in hindsight the documentation really should have been part of #110). Since ASDF-in-Fits support of some fashion is being moved specifically to this package meaning it needs to be documented. This PR is intended to add things to help support the transition of ASDF-in-Fits from asdf to stdatamodels. Documentation of this support is entirely in-scope of these changes IMOP (if you disagree, it still needs to happen and should be a follow-on PR).

A follow-on PR sounds great.

No. the documentation of something, fits_support and/or the "helper functions" for the ASDF-in-Fits transition should live directly alongside the code implementing this. Thus the migration documentation should live here. Insofar as the ASDF documentation, we will need to declare that ASDF-in-Fits is deprecated in the documentation, and then link to the migration guide here using an intersphinx link.

I will work on a module level docstring for asdf_in_fits that describes the migration. Will this be sufficient for the changes you are requesting or did you have more in mind?

[WilliamJamieson]

I will work on a module level docstring for asdf_in_fits that describes the migration. Will this be sufficient for the changes you are requesting or did you have more in mind?

Sure, though it ought to make it into the docs for stdatamodels. Its fine if this goes into the follow-on PR.

[braingram]

I will work on a module level docstring for asdf_in_fits that describes the migration. Will this be sufficient for the changes you are requesting or did you have more in mind?

Sure, though it ought to make it into the docs for stdatamodels. Its fine if this goes into the follow-on PR.

The asdf_in_fits module is already included in the stdatamodels docs through the addition in api.rst. Should this be added somewhere else?

[WilliamJamieson]

The asdf_in_fits module is already included in the stdatamodels docs through the addition in api.rst. Should this be added somewhere else?

I forgot about that. I think that will work, so long as we can create a sphinx link to it.

[braingram]

I added a module docstring to asdf_in_fits. @WilliamJamieson input/edits/suggestions for this are appreciated.

I'm also seeing the built docs on readthedocs with this PR. Thanks @zacharyburnett

[braingram]

@WilliamJamieson Are there additional changes you are requesting?

[braingram]

@zacharyburnett did this miss the 1.0 release? If not, it would be helpful to have this in that release.

@WilliamJamieson pinging you again for clarification on what changes you are requesting.

[braingram]

I rebased this and moved the changes to 1.1.0

[braingram]

The jwst ci job failures that appeared in this PR after updating it:
https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7236004607#step:12:1100
are different than those seen when run against master:
https://github.com/spacetelescope/stdatamodels/actions/runs/4161750811/jobs/7236042033#step:12:622
and will require more investigation.

[zacharyburnett]

It looks like this is just a truth value issue:
https://github.com/spacetelescope/stdatamodels/actions/runs/4161750811/jobs/7249375601#step:12:321

E           AssertionError: 
E           Not equal to tolerance rtol=1e-05, atol=1e-05
E           
E           Mismatched elements: 1 / 3 (33.3%)
E           Max absolute difference: 16.3400141
E           Max relative difference: 0.44103293
E            x: array([[20.709406,  0.46572 ,  4.686621]], dtype=float32)
E            y: array([[37.04942 ,  0.46572 ,  4.686621]])

@braingram I think this is good to merge, and then we can figure out the sticky parts between repos after we release 1.1.0

[braingram]

It looks like this is just a truth value issue: https://github.com/spacetelescope/stdatamodels/actions/runs/4161750811/jobs/7249375601#step:12:321

Thanks for taking a look. I think the run you linked was run against master. The one against this PR has extra errors:
https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7236004607#step:12:1100

  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_entry_points.py::test_get_steps
  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_step.py::test_step_from_commandline_class_alias
  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_step.py::test_step_from_commandline_config_class_alias
  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_utilities.py::test_resolve_step_class_alias

Are those also because of the version issues?

[zacharyburnett]

Thanks for taking a look. I think the run you linked was run against master. The one against this PR has extra errors: https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7236004607#step:12:1100

  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_entry_points.py::test_get_steps
  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_step.py::test_step_from_commandline_class_alias
  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_step.py::test_step_from_commandline_config_class_alias
  FAILED .tox/test-jwst-xdist-cov/lib/python3.11/site-packages/jwst/stpipe/tests/test_utilities.py::test_resolve_step_class_alias

Are those also because of the version issues?

Oh good catch, that's my bad!

https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7236004607#step:12:470

ValueError: 'stpipe_dummy' is not a path to a config file or a Python Step class

https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7236004607#step:12:556

ImportError: stpipe_dummy is not a Python class

https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7236004607#step:12:822

assert resolve_step_class_alias(pipeline_class.class_alias) == full_class_name
AssertionError: assert 'calwebb_ami3' == 'jwst.pipeline.Ami3Pipeline'

I'm not sure what these are, were class names changed?

[braingram]

No class names were changed in this PR.

I think the crds caching isn't configured correctly anymore. The key is crds- instead of something like crds-jwst_1045.pmap
https://github.com/spacetelescope/stdatamodels/actions/runs/4177420337/jobs/7255362938#step:9:4

I pushed a commit re-adding the crds-context id that was removed in 7690332

Any objection if I delete some of the crds- caches and possibly other crds-...pmap caches to see if this is related?

[braingram]

Same errors occur on a test PR with only changes to the readme.
https://github.com/spacetelescope/stdatamodels/actions/runs/4188223854/jobs/7259085190#step:8:1098
(and the PR with the readme changes for reference: #120)

I think that's pretty conclusive that the changes in this PR are not the cause of the jwst test failures.

@zacharyburnett I don't appear to have sufficient permissions to merge this PR without the CI jobs passing. If you're able to do so please feel free.

I tried fixing the crds cache key, and removing the crds cache with no improvement. I will open an issue to track the incorrect cache key.