Add support for specifying output projection as APE 14 WCS with array_shape defined #345
Conversation
Codecov Report
@@ Coverage Diff @@
## main #345 +/- ##
==========================================
+ Coverage 92.39% 92.44% +0.05%
==========================================
Files 24 24
Lines 802 821 +19
==========================================
+ Hits 741 759 +18
- Misses 61 62 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -142,9 +142,11 @@ def parse_output_projection(output_projection, shape_in=None, shape_out=None, ou | |||
| "Need to specify shape since output header " | |||
| "does not contain complete shape information" | |||
| ) | |||
| elif isinstance(output_projection, BaseHighLevelWCS): | |||
| elif isinstance(output_projection, (BaseLowLevelWCS, BaseHighLevelWCS)): | |||
There was a problem hiding this comment.
oh man this change is needed, been meaning to raise an issue for this for ages.
reproject/utils.py
Outdated
| wcs_out = output_projection | ||
| if shape_out is None: | ||
| if getattr(wcs_out, "array_shape") is not None: | ||
| shape_out = wcs_out.array_shape |
There was a problem hiding this comment.
The issue is that not all HighLevelWCS classes also implement the low level API (notably the wrapper doesn't). So I think you might not be able to get to array_shape if the output_projection is an instance of wrapper. (You would have to go through low_level_wcs.
It might be worth standardising on low level?
There was a problem hiding this comment.
We've been allowing high level WCS for a little while so I think we just have to support both (in fact we rely on the high level API internally). I've expanded the test suite and made sure things work for both now.
…_shape defined, and refactor output_projection tests to use a fixture
astrofrog
force-pushed
the
output-projection-ape14-wcs
branch
from
69acba0
to
c2b09db
Compare
This also refactors output_projection tests to use a fixture.
Fixes #309