Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions scopesim/effects/data_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,33 +60,49 @@ class DataContainer:

"""

meta = None

def __init__(self, filename=None, table=None, array_dict=None, cmds=None,
**kwargs):

self.cmds = cmds
if filename is None and "file_name" in kwargs:
warn("The 'file_name' kwarg is deprecated and will raise an error "
"in the future, please use 'filename' instead!",
DeprecationWarning, stacklevel=2)
filename = kwargs["file_name"]

# A !-string filename is immediately resolved, but the file is not yet
# searched for. This makes sense, as the filename should end up in the
# FITS headers, and should therefor be independent of any particular
# system.
#
# It would perhaps be even better to not resolve a !-string filename
# here, but that currently does not make sense because the file is
# directly read in. That is, the file would not be read again if
# someone changes the value that the !-string points to. So changing
# the value a !-string points to would lead to an inconsistent state.
# Immediately resolving the !-string prevents such an inconsistency.
if isinstance(filename, str) and filename.startswith("!"):
filename = utils.from_currsys(filename, self.cmds)

filename = utils.find_file(filename)
self.meta = {
# A derived clas might have set .meta before calling super().__init__()
if self.meta is None:
self.meta = {}
self.meta.update({
"filename": filename,
"description": "",
"history": [],
"name": "<empty>",
}
})
self.meta.update(kwargs)

self.headers = []
self.table = None
self._file = None

if filename is not None:
# Need to check whether the file exists before trying to load it.
filename_full = utils.find_file(self.meta["filename"])
if filename_full is not None:
if self.is_fits:
self._load_fits()
else:
Expand Down Expand Up @@ -116,7 +132,8 @@ def _from_arrays(self, array_dict):
self.table.meta.update(self.meta)

def _load_ascii(self):
self.table = ioascii.read(self.meta["filename"],
filename_full = utils.find_file(self.meta["filename"])
self.table = ioascii.read(filename_full,
format="basic", guess=False)
hdr_dict = utils.convert_table_comments_to_dict(self.table)
if isinstance(hdr_dict, dict):
Expand All @@ -135,7 +152,8 @@ def _load_ascii(self):
f"{self.meta['filename']}"]

def _load_fits(self):
self._file = fits.open(self.meta["filename"])
filename_full = utils.find_file(self.meta["filename"])
self._file = fits.open(filename_full)
for ext in self._file:
self.headers += [ext.header]

Expand Down Expand Up @@ -186,7 +204,8 @@ def is_fits(self):
if isinstance(self._file, fits.HDUList):
flag = True
elif isinstance(self.meta["filename"], str):
flag = utils.is_fits(self.meta["filename"])
filename_full = utils.find_file(self.meta["filename"])
flag = utils.is_fits(filename_full)

return flag

Expand Down
4 changes: 2 additions & 2 deletions scopesim/effects/effects.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class Effect(DataContainer):

"""

def __init__(self, **kwargs):
super().__init__(**kwargs)
def __init__(self, filename=None, **kwargs):
super().__init__(filename=filename, **kwargs)
self.meta["z_order"] = []
self.meta["include"] = True
self.meta.update(kwargs)
Expand Down
12 changes: 8 additions & 4 deletions scopesim/effects/metis_lms_trace_list.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""SpectralTraceList and SpectralTrace for the METIS LM spectrograph."""

import copy
import warnings

import numpy as np
Expand Down Expand Up @@ -307,7 +307,8 @@ def fov_grid(self):
y_min = aperture["bottom"]
y_max = aperture["top"]

layout = ioascii.read(find_file("!DET.layout.filename"))
filename_det_layout = from_currsys("!DET.layout.file_name", cmds=self.cmds)
layout = ioascii.read(find_file(filename_det_layout))
det_lims = {}
xhw = layout["pixel_size"] * layout["x_size"] / 2
yhw = layout["pixel_size"] * layout["y_size"] / 2
Expand Down Expand Up @@ -561,8 +562,11 @@ class MetisLMSEfficiency(TERCurve):
}

def __init__(self, **kwargs):
self.meta = self._class_params
self.meta.update(kwargs)
# TODO: Refactor these _class_params?
self.meta = copy.copy(self._class_params)
assert "grat_spacing" in self.meta, "grat_spacing is missing from self.meta 1"
super().__init__(**kwargs)
assert "grat_spacing" in self.meta, "grat_spacing is missing from self.meta 2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume these assert statements are fine for now, and because this whole class is METIS specific, and nobody is going to screw too much with the current status quo in the METIS IRDB, these will not affect anything down the line. Or?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These asserts should not affect anything. I kept them in because maybe they are a canary that could indicate that something is wrong. What could go wrong is that someone changes __init__() of one of the super classes (e.g. Effect) to delete everything in self.meta. This will highlight if that happens.


filename = find_file(self.meta["filename"])
wcal = fits.getdata(filename, extname="WCAL")
Expand Down
4 changes: 2 additions & 2 deletions scopesim/effects/psf_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ def make_strehl_map_from_table(tbl, pixel_scale=1*u.arcsec):
return map_hdu


def rescale_kernel(image, scale_factor, spline_order=None):
def rescale_kernel(image, scale_factor, spline_order=None, cmds=None):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate why this is here, but I would prefer to keep cmds out of the utils functions. Maybe it's just me being pedantic for no reason, but the util function should be able to be called by anything, even if a cmds object doesn't exist.

I would suggest deleting the call to currsys instead and forcing the offending Effect object to resolve the !-string before calling rescale_kernel

Suggested change
def rescale_kernel(image, scale_factor, spline_order=None, cmds=None):
def rescale_kernel(image, scale_factor, spline_order):

if spline_order is None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if spline_order is None:

spline_order = utils.from_currsys("!SIM.computing.spline_order")
spline_order = utils.from_currsys("!SIM.computing.spline_order", cmds=cmds)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
spline_order = utils.from_currsys("!SIM.computing.spline_order", cmds=cmds)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, if we remove this here, then it is not necessary to add cmds as an argument to rescale_kernel(). I'll update the code in psf.py to get the spline order before calling rescale_kernel().

Note that you can comment on multiple lines at a time by "dragging the plus down" next to the lines in the diff view.

sum_image = np.sum(image)
image = zoom(image, scale_factor, order=spline_order)
image = np.nan_to_num(image, copy=False) # numpy version >=1.13
Expand Down
2 changes: 1 addition & 1 deletion scopesim/effects/psfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ def remake_kernel(self, x):
def wavelength(self):
wave = from_currsys(self.meta["wavelength"], self.cmds)
if isinstance(wave, str) and wave in tu.FILTER_DEFAULTS:
wave = tu.get_filter_effective_wavelength(wave)
wave = tu.get_filter_effective_wavelength(wave, cmds=self.cmds)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here as above. I feel it would be better to force the Effect object to resolve what it needs before calling a util function

Suggested change
wave = tu.get_filter_effective_wavelength(wave, cmds=self.cmds)
wave = tu.get_filter_effective_wavelength(wave)

wave = quantify(wave, u.um).value

return wave
Expand Down
16 changes: 8 additions & 8 deletions scopesim/effects/ter_curves.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ class TERCurve

"""

def __init__(self, **kwargs):
super().__init__(**kwargs)
def __init__(self, filename=None, **kwargs):
super().__init__(filename=filename, **kwargs)
params = {
"z_order": [10, 110, 510],
"ignore_wings": False,
Expand Down Expand Up @@ -398,7 +398,7 @@ def __init__(self, cmds=None, **kwargs):
"(`filename`, `array_dict`, `table`) or both "
f"(`filter_name`, `filename_format`): {kwargs}")

super().__init__(**kwargs)
super().__init__(cmds=cmds, **kwargs)
if self.table is None:
raise ValueError("Could not initialise filter. Either filename "
"not found, or array are not compatible")
Expand Down Expand Up @@ -872,7 +872,7 @@ def __init__(self, transmission, cmds=None, **kwargs):
self.cmds = cmds
wave_min = from_currsys(self.params["wave_min"], self.cmds) * u.um
wave_max = from_currsys(self.params["wave_max"], self.cmds) * u.um
transmission = from_currsys(transmission)
transmission = from_currsys(transmission, cmds=self.cmds)

super().__init__(wavelength=[wave_min, wave_max],
transmission=[transmission, transmission],
Expand Down Expand Up @@ -901,8 +901,8 @@ class ADCWheel(Effect):
required_keys = {"adc_names", "filename_format", "current_adc"}
_current_str = "current_adc"

def __init__(self, **kwargs):
super().__init__(**kwargs)
def __init__(self, cmds=None, **kwargs):
super().__init__(cmds=cmds, **kwargs)
check_keys(kwargs, self.required_keys, action="error")

params = {"z_order": [125, 225, 525],
Expand All @@ -915,7 +915,7 @@ def __init__(self, **kwargs):

path = self._get_path()
self.adcs = {}
for name in from_currsys(self.meta["adc_names"]):
for name in from_currsys(self.meta["adc_names"], cmds=self.cmds):
kwargs["name"] = name
self.adcs[name] = TERCurve(filename=str(path).format(name),
**kwargs)
Expand All @@ -937,7 +937,7 @@ def change_adc(self, adcname=None):
@property
def current_adc(self):
"""Return the currently used ADC."""
curradc = from_currsys(self.meta["current_adc"])
curradc = from_currsys(self.meta["current_adc"], cmds=self.cmds)
if not curradc:
return False
return self.adcs[curradc]
Expand Down
4 changes: 2 additions & 2 deletions scopesim/effects/ter_curves_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@
PATH_SVO_DATA = PATH_HERE.parent / "data" / "svo"


def get_filter_effective_wavelength(filter_name):
def get_filter_effective_wavelength(filter_name, cmds=None):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before, lets delete the currsys call

Suggested change
def get_filter_effective_wavelength(filter_name, cmds=None):
def get_filter_effective_wavelength(filter_name):

if isinstance(filter_name, str):
filter_name = from_currsys(filter_name)
filter_name = from_currsys(filter_name, cmds=cmds)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
filter_name = from_currsys(filter_name, cmds=cmds)
assert FILTER_DEFAULTS.get(filter_name), f"{filter_name} not found in FILTER_DEFAULTS"

wave, trans = download_svo_filter(FILTER_DEFAULTS[filter_name],
return_style="quantity")
eff_wave = np.sum(wave * trans) / np.sum(trans) # convert from Angstrom
Expand Down
4 changes: 2 additions & 2 deletions scopesim/optics/fov.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,10 @@ def make_cube_hdu(self):

# 1. Make waveset and canvas cube (area, bin_width are applied at end)
# TODO: Why is this not self.waveset? What's different?
wave_unit = u.Unit(from_currsys("!SIM.spectral.wave_unit"), self.cmds)
wave_unit = u.Unit(from_currsys("!SIM.spectral.wave_unit", self.cmds))
fov_waveset = np.arange(
self.meta["wave_min"].value, self.meta["wave_max"].value,
from_currsys("!SIM.spectral.spectral_bin_width"), self.cmds) * wave_unit
from_currsys("!SIM.spectral.spectral_bin_width", self.cmds)) * wave_unit
fov_waveset = fov_waveset.to(u.um)

# TODO: what's with this code??
Expand Down
2 changes: 1 addition & 1 deletion scopesim/optics/optical_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ def prepare_source(self, source):
# Put on fov wavegrid
wave_min = min(fov.meta["wave_min"] for fov in self.fov_manager.fovs)
wave_max = max(fov.meta["wave_max"] for fov in self.fov_manager.fovs)
wave_unit = u.Unit(from_currsys("!SIM.spectral.wave_unit"), self.cmds)
wave_unit = u.Unit(from_currsys("!SIM.spectral.wave_unit", self.cmds))
dwave = from_currsys("!SIM.spectral.spectral_bin_width", self.cmds) # Not a quantity
fov_waveset = np.arange(wave_min.value, wave_max.value, dwave) * wave_unit
fov_waveset = fov_waveset.to(u.um)
Expand Down