Add support for HighLevelWCSWrapper

[Cadair]

This is used quite heavily in ndcube.

I added use of the lazy fixtures package here, the test changes for SlicedLowLevelWCS are kind of related to #276 in that it makes it easy to also test with gwcs.

[Cadair]

It would be really good to have a way of saying "Any APE-14 compliant WCS". I am kind of tempted to allow any object inside these WCS wrapper classes as we can't possibly enumerate all of them?

Now I think about it #276 has the same problem with the changes to the SlicedLowLeveWCS schema, we should be allowing all the NDCube wrappers there as well.

[codecov]

Codecov Report

❌ Patch coverage is 96.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.75%. Comparing base (97556cc) to head (f46c8e5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...y/converters/wcs/tests/test_highlevelwcswrapper.py	94.44%	2 Missing ⚠️
asdf_astropy/testing/helpers.py	90.90%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (97556cc) and HEAD (f46c8e5). Click for more details.

HEAD has 5 uploads less than BASE

Flag	BASE (97556cc)	HEAD (f46c8e5)
	8	3

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #296       +/-   ##
===========================================
- Coverage   99.39%   88.75%   -10.65%     
===========================================
  Files          65       68        +3     
  Lines        2326     2410       +84     
===========================================
- Hits         2312     2139      -173     
- Misses         14      271      +257     

☔ 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.

[Cadair]

Is this oldestdeps fail an asdf standard < 1.6 error? What's the best solution?

[Cadair]

I had to add this here to make oldestdeps work. I'm not sure if it's a good idea or not though?

[braingram]

Let's not change this and address the failures a different way. Do you have a link to the failures?

[braingram]

Is there a public way to get the low level wcs?

[braingram]

More of a general question but what's the expectation here for HighLevelWCSWrapper? I'm not familiar with it but from the docstring and code (and name) it wraps a "low level" WCS and provides a different API. Structurally the only important information appears to be:

• we have an instance of a HighLevelWCSWrapper
• it has a low_level_wcs

I think the schema in this PR and the serialization structure are an accurate representation of that information. Given the relatively simple structure I'm wondering what benefit there is in all of the tests with GWCS instances. Does gwcs (or astropy) not already have tests that a GWCS instance can be used with HighLevelWCSWrapper? If not those tests would fit better in a different package. If so I question if we need to test that here. For constructing a HighLevelWCSWrapper we could just use an astropy WCS. Is there some nuance with HighLevelWCSWrapper and GWCS instances that wouldn't be covered by this option (dropping gwcs usage in this PR)?

[Cadair]

gwcs.WCS and astropy.wcs.WCS both implement the high and low level APIs. Things such as the wrapper classes only implement the low level api so the high level wrapper gives you a way to get a high level object.

Yes probably we don't strictly need to test all combinations of things if it feels lile overkill 😆

[braingram]

gwcs.WCS and astropy.wcs.WCS both implement the high and low level APIs. Things such as the wrapper classes only implement the low level api so the high level wrapper gives you a way to get a high level object.

Yes probably we don't strictly need to test all combinations of things if it feels lile overkill 😆

Thanks for the clarification. That sounds good to me. Would you update this PR dropping gwcs as a test dependency and making the required test changes?

[Cadair]

Actually now I think about it, we don't test that gwcs.WCS is allowed in #276 for SlicedLowLevelWCS either. I kinda feel like if we have these wrapper classes which should allow serialising a wrapped gwcs object we should test it?

[braingram]

Actually now I think about it, we don't test that gwcs.WCS is allowed in #276 for SlicedLowLevelWCS either. I kinda feel like if we have these wrapper classes which should allow serialising a wrapped gwcs object we should test it?

Thanks. I think the ideal situation would be that we could add gwcs as a test dependency and then test these cases (even though they're largely duplicating tests that hopefully exist already in gwcs). However adding gwcs as a test dependency is problematic since it has different minimum requirements and certain versions won't work here.

I'm not opposed to either:

• not adding these wrapped gwcs tests (and not needing to add gwcs as a test dependency)
• adding gwcs as a test dependency and skipping the gwcs-related testing for the minimum-deps run (perhaps similar to what's done in Test PR #298)

Which would you prefer?