Fix Errors in computation in ResampledLowLevelWCS #857
Conversation
Co-authored-by: Dan Ryan <[email protected]>
Co-authored-by: Dan Ryan <[email protected]>
Explains how the test cases work. Also use numpy.linspace instead of numpy.arange in formulating expected results to make it more easily read.
… cadair_fix_resampled
Also added a Notes section to that docstring with a graphical example
…WCS docstring for greater clarity.
| The location on the underlying pixel grid which corresponds | ||
| to zero on the top level pixel grid. If a scalar, the grid will be | ||
| shifted by the same amount in all dimensions. |
There was a problem hiding this comment.
I think this might actually be the source of the "bug". We previously defined offset to be the location of the centre of the 0th pixel after resampling. This means that the location of the left edge (-0.5) is not simply determined by offset but by offset - factor/2. Arguably this is a breaking behaviour change rather than a bugfix. But I also think the original behaviour is highly non-intuitive, so in practice feels like a bug.
There was a problem hiding this comment.
I think it's a bug fix in that we can define it like that as much as we want but that's not how we were using it in rebin 😆
|
@Cadair: This is now ready for review. (Hopefully someone can point out why the oldestdeps and figure tests are failing.) |
Cadair
force-pushed
the
fix_resampled
branch
from
ae22d23 to
8de296d
Compare
| @@ -0,0 +1 @@ | |||
| Fix translations in `~ndcube.wcs.wrappers.resampled_wcs.ResampledLowLevelWCS` between original and resampled pixel grids. | |||
There was a problem hiding this comment.
I am assuming this was manifesting as errors in accuracy in rebin, we should probably call that out too?
Cadair
changed the title
To Do
test_scalar_factortest.test_2dtest.test_rebintest.ResampledLowLevelWCSclass.