scopesim.Source fails with cube filename input and tabular WCS
Opened this issue · 7 comments
This is from the example notebook LSS_AGN-01_preparation.ipynb
in irdb/METIS
, using scopesim 0.9.0a6:
src = sim.Source(cube="AGN_sim_prepared.fits")
...
File ~/ELT_Development/ScopeSim/scopesim/source/source_fields.py:283, in CubeSourceField.__post_init__(self)
281 """Validate input."""
282 if self.wcs is None and not self.from_hdul:
--> 283 self.wcs = WCS(self.field)
...
ValueError: HDUList is required to retrieve -TAB coordinates and/or indices.
Obviously, self.field
has lost the WCS-TAB
extension that is in the fits file. I don't know what self.from_hdul
is, would that help?
I thought I had specifically addressed TAB WCS cubes in my source fields change. Obviously not very well, at least for this case. I'll take a look (probably tomorrow), I should know where to look for this...
I think I remember that it works if you open the file first and then pass the HDUL instead, but can't check now. Feel free to try that 🙂
Hang on, I actually did that in this notebook! Which branch are you on for the IRDB?
I guess I haven't used the latest version of the notebook. That sounds a bit hacky, though.
Yeah I agree that filename should also work (again). I think I made this change after the whole source fields stuff broke it and this was easier than going back there. But I should implement a proper fix, it shouldn't be that hard...
Looking at the current implementation, when Source._from_cube()
(which should really be a classmethod constructor imo) receives as filename, only the extension specified by the optional ext
argument is loaded, which defaults to 0. I could modify it such that loading a multi-extension FITS file will always trigger the same behavior as supplying the corresponding HDUL directly. However, that might break some code that implicitly expects that only the zeroth HDU is loaded.
My suggestion to properly solve this:
- Allow
ext=None
which loads all extensions, soSource(cube="AGN_sim_prepared.fits", ext=None)
would work. - Keep the default
ext=0
to preserve backwards compatibility, but log a warning that this will change if noext
is supplied. - Change the default to
ext=None
in the next minor version and explicitly supplyext=0
in all the cases where that's what we actually want in our own code and notebooks.
Addendum: It is possible to load a TAB WCS from another HDU even with a certain ext
set, see the current implementation of CubeSourceField.from_hdulist()
. So we might actually be fine with keeping the default and just always treat the filename case identically to the HDUL case. After all, even with a TAB WCS, ScopeSim still needs to know in which HDU the actual cube data is. So maybe what I wrote in the comment above isn't such a good idea after all...