Bounding box refactor support

[WilliamJamieson]

This PR enables GWCS to support the recently merged PR astropy/astropy#11930 which refactors how astropy.modeling handles bounding boxes. In particular this PR removes the private class astropy.modeling.utils._BoundingBox and replaces it with the astropy.modeling.bounding_box.BoundingBox class while retaining nearly identical functionality.

This PR primarily switches GWCS over to using this new class. Additionally, it implements the new feature from BoundingBox which allows users to specify the storage ordering of bounding box intervals. This removes the need for reversal of bounding box orderings within GWCS under some circumstances by allowing astropy to automatically detect and handle these circumstances.

[pllim]

Shouldn't you be using astropy version check with this patch? That way in the future, when your astropy minversion is 5.0, you can drop the old compatibility code.

Also, I was using gwcs with dev astropy, and that broke my code downstream in Jdaviz (spacetelescope/jdaviz#938). I don't know if there is any good fix for this except to tell each user to update gwcs and their code to be compatible with astropy/astropy#11930 ... but I have a feeling I won't be the only one affected here. 🤷

.../jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py in vue_do_aper_phot(self, *args, **kwargs)
    167                  'ycenter': reg.center.y * u.pix}
    168             if data.coords is not None:
--> 169                 d['sky_center'] = data.coords.pixel_to_world(reg.center.x, reg.center.y)
    170             else:
    171                 d['sky_center'] = None

.../gwcs/api.py in pixel_to_world(self, *pixel_arrays)
    297         """
    298         pixels = self._sanitize_pixel_inputs(*pixel_arrays)
--> 299         return self(*pixels, with_units=True)
    300 
    301     def array_index_to_world(self, *index_arrays):

.../gwcs/wcs.py in __call__(self, *args, **kwargs)
    312             kwargs['fill_value'] = np.nan
    313 
--> 314         if self.bounding_box is not None:
    315             # Currently compound models do not attempt to combine individual model
    316             # bounding boxes. Get the forward transform and assign the bounding_box to it

.../gwcs/wcs.py in bounding_box(self)
   1272             axes_order = np.arange(transform_0.n_inputs)
   1273         # Model.bounding_box is in python order, need to reverse it first.
-> 1274         return tuple(bb[::-1][i] for i in axes_order)
   1275 
   1276     @bounding_box.setter

.../gwcs/wcs.py in <genexpr>(.0)
   1272             axes_order = np.arange(transform_0.n_inputs)
   1273         # Model.bounding_box is in python order, need to reverse it first.
-> 1274         return tuple(bb[::-1][i] for i in axes_order)
   1275 
   1276     @bounding_box.setter

.../astropy/modeling/bounding_box.py in __getitem__(self, key)
    235     def __getitem__(self, key):
    236         """Get bounding_box entries by either input name or input index"""
--> 237         return self._intervals[self._get_index(key)]
    238 
    239     def _get_order(self, order: str = None) -> str:

.../astropy/modeling/bounding_box.py in _get_index(self, key)
    220                 raise IndexError(f"Integer key: {key} must be < {len(self._model.inputs)}.")
    221         else:
--> 222             raise ValueError(f"Key value: {key} must be string or integer.")
    223 
    224         return index

ValueError: Key value: slice(None, None, -1) must be string or integer.

Maybe a clearer error message would help. Regardless I think a memo to astropy-dev mailing list is necessary.

[codecov]

Codecov Report

Merging #376 (f500688) into master (2d1c1ed) will decrease coverage by 0.25%.
The diff coverage is 70.27%.

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
- Coverage   86.34%   86.08%   -0.26%     
==========================================
  Files          21       21              
  Lines        3624     3643      +19     
==========================================
+ Hits         3129     3136       +7     
- Misses        495      507      +12     

Impacted Files	Coverage Δ
gwcs/wcs.py	88.48% <70.27%> (-0.97%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d1c1ed...f500688. Read the comment docs.

[WilliamJamieson]

Shouldn't you be using astropy version check with this patch? That way in the future, when your astropy minversion is 5.0, you can drop the old compatibility code.

miniversion in this case can be a bit contrary if one has not updated astropy right now as the best way to test if one is using 5.0 using minversion right now is minversion('astropy', '5.0.dev'). A better solution right now is to just test for the existence of astropy.modeling.bounding_box as I am doing right now with my backwards compatibility commit.

[pllim]

Re: minversion -- As long as you (as a team) have a way to search for these old code and remove them when they are no longer necessary. You might not be the one who will do the update. I'll leave that to your good judgement.

[nden]

The old code may be necessary for a very long time in order to be able to read files with the old WCS stored in them.

[nden]

I will merge it as is to fix failing tests in jwst but it needs a Change log .