Don't create a whole new map just to get a WCS - [closed]

[nabobalis]

In GitLab by @dstansby1 on Oct 29, 2021, 13:12

Merges spike-removal -> master

Fixes https://gitlab.com/LMSAL_HUB/aia_hub/aiapy/-/issues/121. Admittedly it doesn't track down what causes the original bug, but this is a much more efficient way of doing things anyway.

[nabobalis]

I wonder if we need to add a test for doing fetch_spikes to a submap?

Edit: I tried doing it with the example we have and it didn't error. I wrote a test with the file I used from the blog post and that replicated it and this PR did it fix.

The issue seems to be the one test failure which is real.

[nabobalis]

I think the corrections/changes that map.wcs does to the WCS on its return is important but I don't quite understand why.

[nabobalis]

In GitLab by @wtbarnes on Oct 29, 2021, 18:05

FWIW, there is already a test for the submap case: https://gitlab.com/LMSAL_HUB/aia_hub/aiapy/-/blob/master/aiapy/calibrate/tests/test_spikes.py#L54

[nabobalis]

The issue I hit was when I called fetch_spike on the submap.

Replicating it has been tricky for me.

[nabobalis]

In GitLab by @wtbarnes on Nov 1, 2021, 07:22

There is a reason I went through the full map instantiation and didn't do it this way. In sunpy, we don't seem to have a good way of reproducing the wcs property of a map outside of creating a map instance. I'll note that this PR does actually solve the test failure in #121, but now, because the WCS here is not actually the full-frame equivalent of the submap, the equivalency test for the submap case fails.

[nabobalis]

In GitLab by @wtbarnes on Nov 3, 2021, 11:04

So I think what we should be doing here is modifying the WCS of the submap rather than creating a whole new map.

[nabobalis]

Closed in favour of !139