Add basic sky coordinates (WCS) to ScopeSim output

[teutoburg]

1. Add two functions in optics/image_plane_utils: det_wcs_from_sky_wcs() and sky_wcs_from_det_wcs(), which convert detector (mm) WCS into on-sky (arcsec or deg) WCS and vice versa using the pixel and plate scales. Both in turn use the function create_wcs_from_points(), which has been slightly modified as well. There used to be a bit of a less-than-ideal state because a sky WCS could use either arcsec or deg and ScopeSim is not fully consistent with that, which may lead to bugs if one or the other is incorrectly assumed. All three functions now use astropy units, which is also not ideal for performance, but acceptable for now until we can be certain our units are consistent. If a non-quantity (i.e. float) is passed, a reasonable (documented) unit is assumed. The function header_from_list_of_xy(), which internally also calls create_wcs_from_points(), is unchanged in its parameters and still uses the arcsec flag. This function might be phased out mostly in favor of direct calls to create_wcs_from_points().
2. Adaptions to these changes in several places, most importantly in FOVManager.generate_fov_list(), which now uses det_wcs_from_sky_wcs(). The code previously there was the main inspiration for the two new functions.
3. Include the (approximate) on-sky WCS in Detector.hdu, which was the main motivation to do all of this. This will be collected by DetectorArray.readout() and thus also included in the output from OpticalTrain.readout(). In the process, some minor changes to DetectorArray as a whole, mostly documentation, some comments, and everyone's favorite: formatting and linting 🙃
4. Remove some obsolete lines from OpticalTrain.write_header(), notably the WCS and some input file names, see also Deal with header keywords #305 .
5. Some minor formatting and cleanup in a few other places.
6. Add tests for the new WCS functions to ensure a round-trip of sky ➡️ detector ➡️ sky works.

[codecov]

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (262e894) 80.07% compared to head (69fa9a8) 80.28%.

Files	Patch %	Lines
scopesim/detector/detector_array.py	85.71%	1 Missing ⚠️
scopesim/effects/psf_utils.py	0.00%	1 Missing ⚠️
scopesim/optics/fov.py	90.00%	1 Missing ⚠️
scopesim/optics/image_plane_utils.py	97.91%	1 Missing ⚠️
scopesim/optics/optical_train.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff               @@
##           dev_master     #307      +/-   ##
==============================================
+ Coverage       80.07%   80.28%   +0.20%     
==============================================
  Files             147      147              
  Lines           14819    14844      +25     
==============================================
+ Hits            11866    11917      +51     
+ Misses           2953     2927      -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hugobuddel]

Great work @teutoburg !

Did you test the IRDB against this branch? I'm a bit worried that this change might break some expectations in the IRDB. But nevertheless I think it is okay to merge this and tackle the problems afterwards. We should be ready for that though.

What still is missing is that the WCS coordinates are not yet added to the headers if any fits header Effects are defined. That is fine for now, as we haven't really defined how to proceed, but all options require this cleanup first and we shouldn't put everything in one PR.

[teutoburg]

Did you test the IRDB against this branch? I'm a bit worried that this change might break some expectations in the IRDB. But nevertheless I think it is okay to merge this and tackle the problems afterwards. We should be ready for that though.

Didn't do that yet, but planning to. As you say, should be fine to do it after merging (but before any new release).

What still is missing is that the WCS coordinates are not yet added to the headers if any fits header Effects are defined. That is fine for now, as we haven't really defined how to proceed, but all options require this cleanup first and we shouldn't put everything in one PR.

Yes, that was intentional, to keep things separate that are technically separate. If one or the other ends up breaking something, it'll be easier to deal with it I guess.

[teutoburg]

Yes, that was intentional, to keep things separate that are technically separate. If one or the other ends up breaking something, it'll be easier to deal with it I guess.

Small correction, the WCS should be included anyway, as it's created directly in the Detector. Unless there is some overwriting going on that I'm unaware of...