AstarVienna/ScopeSim

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, so Source(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 no ext is supplied.
  • Change the default to ext=None in the next minor version and explicitly supply ext=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...