Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not set user commands as rc.__currsys__ #377

Merged
merged 13 commits into from
Feb 29, 2024
6 changes: 6 additions & 0 deletions scopesim/effects/data_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ class DataContainer:
def __init__(self, filename=None, table=None, array_dict=None, cmds=None,
**kwargs):
self.cmds = cmds
# Setting a default for cmds cannot be done here, because from_currsys
# checks whether cmds is None. TODO: make this possible.
# if self.cmds is None:
# from scopesim import UserCommands
# self.cmds = UserCommands()

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!",
Expand Down
11 changes: 7 additions & 4 deletions scopesim/effects/electronic.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@

self.meta["detector_readout_mode"] = mode_name
props_dict = self.mode_properties[mode_name]
rc.__currsys__["!OBS.detector_readout_mode"] = mode_name
self.cmds["!OBS.detector_readout_mode"] = mode_name

Check warning on line 108 in scopesim/effects/electronic.py

View check run for this annotation

Codecov / codecov/patch

scopesim/effects/electronic.py#L108

Added line #L108 was not covered by tests
for key, value in props_dict.items():
rc.__currsys__[key] = value
self.cmds[key] = value

Check warning on line 110 in scopesim/effects/electronic.py

View check run for this annotation

Codecov / codecov/patch

scopesim/effects/electronic.py#L110

Added line #L110 was not covered by tests

return obj

Expand Down Expand Up @@ -185,6 +185,9 @@
params = {"z_order": [902]}
self.meta.update(params)
self.meta.update(kwargs)
if self.cmds is None:
from scopesim import UserCommands
self.cmds = UserCommands()

required_keys = ["fill_frac", "full_well", "mindit"]
check_keys(self.meta, required_keys, action="error")
Expand Down Expand Up @@ -226,8 +229,8 @@
logger.info("Exposure parameters: DIT=%.3f s NDIT=%d", dit, ndit)
logger.info("Total exposure time: %.3f s", dit * ndit)

rc.__currsys__["!OBS.dit"] = dit
rc.__currsys__["!OBS.ndit"] = ndit
self.cmds["!OBS.dit"] = dit
self.cmds["!OBS.ndit"] = ndit

return obj

Expand Down
6 changes: 3 additions & 3 deletions scopesim/effects/fits_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ class EffectsMetaKeywords(ExtraFitsKeywords):
"""

def __init__(self, cmds=None, **kwargs):
super(ExtraFitsKeywords, self).__init__()
super(ExtraFitsKeywords, self).__init__(cmds=cmds, **kwargs)
params = {"name": "effects_fits_keywords",
"description": "Effect Meta FITS headers",
"z_order": [998],
Expand Down Expand Up @@ -512,7 +512,7 @@ class SourceDescriptionFitsKeywords(ExtraFitsKeywords):
"""

def __init__(self, cmds=None, **kwargs):
super(ExtraFitsKeywords, self).__init__()
super(ExtraFitsKeywords, self).__init__(cmds=cmds, **kwargs)
params = {"name": "source_fits_keywords",
"description": "Source description FITS headers",
"z_order": [997],
Expand Down Expand Up @@ -596,7 +596,7 @@ class SimulationConfigFitsKeywords(ExtraFitsKeywords):
"""

def __init__(self, cmds=None, **kwargs):
super(ExtraFitsKeywords, self).__init__()
super(ExtraFitsKeywords, self).__init__(cmds=cmds, **kwargs)
params = {"name": "simulation_fits_keywords",
"description": "Simulation Config FITS headers",
"z_order": [996],
Expand Down
4 changes: 1 addition & 3 deletions scopesim/effects/psf_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ def make_strehl_map_from_table(tbl, pixel_scale=1*u.arcsec):
return map_hdu


def rescale_kernel(image, scale_factor, spline_order=None, cmds=None):
if spline_order is None:
spline_order = utils.from_currsys("!SIM.computing.spline_order", cmds=cmds)
def rescale_kernel(image, scale_factor, spline_order):
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
9 changes: 6 additions & 3 deletions scopesim/effects/psfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,8 @@ 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, cmds=self.cmds)
filter_name = from_currsys(wave, cmds=self.cmds)
wave = tu.get_filter_effective_wavelength(filter_name)
wave = quantify(wave, u.um).value

return wave
Expand Down Expand Up @@ -634,7 +635,8 @@ def get_kernel(self, fov):
# rescaling kept inside loop to avoid rescaling for every fov
pix_ratio = kernel_pixel_scale / fov_pixel_scale
if abs(pix_ratio - 1) > self.meta["flux_accuracy"]:
self.kernel = pu.rescale_kernel(self.kernel, pix_ratio)
spline_order = from_currsys("!SIM.computing.spline_order", cmds=self.cmds)
self.kernel = pu.rescale_kernel(self.kernel, pix_ratio, spline_order)

if ((fov.header["NAXIS1"] < hdr["NAXIS1"]) or
(fov.header["NAXIS2"] < hdr["NAXIS2"])):
Expand Down Expand Up @@ -830,8 +832,9 @@ def get_kernel(self, fov):
# rescale the pixel scale of the kernel to match the fov images
pix_ratio = fov_pixel_scale / kernel_pixel_scale
if abs(pix_ratio - 1) > self.meta["flux_accuracy"]:
spline_order = from_currsys("!SIM.computing.spline_order", cmds=self.cmds)
for ii, kern in enumerate(self.kernel):
self.kernel[ii][0] = pu.rescale_kernel(kern[0], pix_ratio)
self.kernel[ii][0] = pu.rescale_kernel(kern[0], pix_ratio, spline_order)

for i, kern in enumerate(self.kernel):
self.kernel[i][0] /= np.sum(kern[0])
Expand Down
3 changes: 2 additions & 1 deletion scopesim/effects/ter_curves.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def __init__(self, filename=None, **kwargs):
self.meta.update(params)
self.meta.update(kwargs)

self.surface = SpectralSurface()
self.surface = SpectralSurface(cmds=self.cmds)
self.surface.meta.update(self.meta)
self._background_source = None

Expand Down Expand Up @@ -918,6 +918,7 @@ def __init__(self, cmds=None, **kwargs):
for name in from_currsys(self.meta["adc_names"], cmds=self.cmds):
kwargs["name"] = name
self.adcs[name] = TERCurve(filename=str(path).format(name),
cmds=cmds,
**kwargs)

self.table = self.get_table()
Expand Down
2 changes: 1 addition & 1 deletion scopesim/effects/ter_curves_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@

def get_filter_effective_wavelength(filter_name, cmds=None):
if isinstance(filter_name, str):
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
9 changes: 7 additions & 2 deletions scopesim/optics/fov_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,13 @@ def generate_fovs_list(self):
det_eff = eu.get_all_effects(self.effects, DetectorList)[0]
dethdr = det_eff.image_plane_header

fovs.append(FieldOfView(skyhdr, waverange, detector_header=dethdr,
**vol["meta"]))
fovs.append(FieldOfView(
skyhdr,
waverange,
detector_header=dethdr,
cmds=self.cmds,
**vol["meta"],
))

return fovs

Expand Down
5 changes: 4 additions & 1 deletion scopesim/optics/optical_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ def load(self, user_commands):
# UserCommands via rc.__currsys__, but on the other hand some
# tests (now with proper patching) fail because of this type
# change. THIS IS A PROBLEM!
rc.__currsys__ = user_commands
# NOTE: All tests pass without setting rc.__currsys__ to user_commands.
# Nevertheless, I'm a bit reluctant to removing this code just
# yet. So it is commented out.
# rc.__currsys__ = user_commands
teutoburg marked this conversation as resolved.
Show resolved Hide resolved
self.yaml_dicts = self.cmds.yaml_dicts
self.optics_manager = OpticsManager(self.yaml_dicts, self.cmds)
self.update()
Expand Down
74 changes: 37 additions & 37 deletions scopesim/tests/tests_effects/test_AutoExposure.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from scopesim import UserCommands
from scopesim.optics.image_plane import ImagePlane
from scopesim.effects.electronic import AutoExposure
from scopesim.utils import from_currsys

from scopesim.tests.mocks.py_objects.imagehdu_objects import _image_hdu_square

Expand All @@ -28,7 +27,7 @@ def fixture_imageplane():
return implane


@pytest.fixture(name="autoexposure", scope="class")
@pytest.fixture(name="autoexposure", scope="function")
def fixture_autoexposure():
"""Instantiate an AutoExposure object"""
return AutoExposure(fill_frac=0.75,
Expand All @@ -45,8 +44,8 @@ def test_initialises_correctly(self):
assert isinstance(autoexposure, AutoExposure)

def test_returns_imageplane(self, autoexposure, imageplane):
with patch("scopesim.rc.__currsys__", _patched_cmds()):
outimpl = autoexposure.apply_to(imageplane)
autoexposure.cmds = _patched_cmds()
outimpl = autoexposure.apply_to(imageplane)

assert isinstance(outimpl, ImagePlane)

Expand All @@ -59,24 +58,24 @@ def test_produces_correct_values(self, autoexposure, imageplane):
ref_dit = 0.75

# TODO: Change AutoExposure to read exptime like dit and ndit
with patch("scopesim.rc.__currsys__", _patched_cmds(exptime=exptime)):
autoexposure.apply_to(imageplane)
autoexposure.cmds = _patched_cmds(exptime=exptime)
autoexposure.apply_to(imageplane)

out_dit = from_currsys("!OBS.dit")
out_ndit = from_currsys("!OBS.ndit")
out_dit = autoexposure.cmds["!OBS.dit"]
out_ndit = autoexposure.cmds["!OBS.ndit"]

assert out_dit == pytest.approx(ref_dit)
assert out_dit * out_ndit == pytest.approx(exptime)

def test_detects_saturation(self, imageplane):
mindit = 0.011
with patch("scopesim.rc.__currsys__", _patched_cmds(exptime=100.)):
autoexposure = AutoExposure(fill_frac=0.75,
full_well=10.,
mindit=mindit)
autoexposure.apply_to(imageplane)
autoexposure = AutoExposure(fill_frac=0.75,
full_well=10.,
mindit=mindit,
cmds=_patched_cmds(exptime=100.))
autoexposure.apply_to(imageplane)

out_dit = from_currsys("!OBS.dit")
out_dit = autoexposure.cmds["!OBS.dit"]

assert out_dit == mindit

Expand All @@ -85,19 +84,19 @@ def test_fill_frac_acts_correctly(self, imageplane):
fill_2 = 0.5
autoexp_1 = AutoExposure(fill_frac=fill_1,
full_well=1e5,
mindit=0.011)
with patch("scopesim.rc.__currsys__", _patched_cmds()):
autoexp_1.apply_to(imageplane)
out_dit_1 = from_currsys("!OBS.dit")
out_ndit_1 = from_currsys("!OBS.ndit")
mindit=0.011,
cmds=_patched_cmds())
autoexp_1.apply_to(imageplane)
out_dit_1 = autoexp_1.cmds["!OBS.dit"]
out_ndit_1 = autoexp_1.cmds["!OBS.ndit"]

autoexp_2 = AutoExposure(fill_frac=fill_2,
full_well=1e5,
mindit=0.011)
with patch("scopesim.rc.__currsys__", _patched_cmds()):
autoexp_2.apply_to(imageplane)
out_dit_2 = from_currsys("!OBS.dit")
out_ndit_2 = from_currsys("!OBS.ndit")
mindit=0.011,
cmds=_patched_cmds())
autoexp_2.apply_to(imageplane)
out_dit_2 = autoexp_2.cmds["!OBS.dit"]
out_ndit_2 = autoexp_2.cmds["!OBS.ndit"]

assert out_dit_1 == fill_1 / fill_2 * out_dit_2
assert out_ndit_1 == fill_2 / fill_1 * out_ndit_2
Expand All @@ -108,17 +107,18 @@ def test_exptime_specified_by_dit_ndit(self, autoexposure, imageplane):
instead of `!OBS.exptime`.
"""
# 1. use exptime
with patch("scopesim.rc.__currsys__", _patched_cmds(exptime=10.)):
autoexposure.apply_to(imageplane)
dit_1 = from_currsys("!OBS.dit")
ndit_1 = from_currsys("!OBS.ndit")
autoexposure.cmds["!OBS.exptime"] = 10.0
autoexposure.apply_to(imageplane)
dit_1 = autoexposure.cmds["!OBS.dit"]
ndit_1 = autoexposure.cmds["!OBS.ndit"]

# 2. use dit and ndit
with patch("scopesim.rc.__currsys__",
_patched_cmds(exptime=None, dit=5, ndit=2)):
autoexposure.apply_to(imageplane)
dit_2 = from_currsys("!OBS.dit")
ndit_2 = from_currsys("!OBS.ndit")
autoexposure.cmds["!OBS.exptime"] = None
autoexposure.cmds["!OBS.dit"] = 5
autoexposure.cmds["!OBS.ndit"] = 2
autoexposure.apply_to(imageplane)
dit_2 = autoexposure.cmds["!OBS.dit"]
ndit_2 = autoexposure.cmds["!OBS.ndit"]

assert dit_1 == dit_2
assert ndit_1 == ndit_2
Expand All @@ -129,10 +129,10 @@ def test_exptime_at_least_mindit(self, imageplane):
autoexposure = AutoExposure(fill_frac=0.75,
full_well=1e5,
mindit=mindit)
with patch("scopesim.rc.__currsys__", _patched_cmds(exptime=exptime)):
autoexposure.apply_to(imageplane)
dit = from_currsys("!OBS.dit")
ndit = from_currsys("!OBS.ndit")
autoexposure.cmds["!OBS.exptime"] = exptime
autoexposure.apply_to(imageplane)
dit = autoexposure.cmds["!OBS.dit"]
ndit = autoexposure.cmds["!OBS.ndit"]

assert dit == mindit
assert ndit == 1
9 changes: 5 additions & 4 deletions scopesim/tests/tests_effects/test_fits_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,11 @@ def test_works(self, simplecado_opt):
}
}
}
extra_keys = fh.ExtraFitsKeywords(header_dict=hdr_dic)
opt_keys = fh.EffectsMetaKeywords()
src_keys = fh.SourceDescriptionFitsKeywords()
config_keys = fh.SimulationConfigFitsKeywords()
cmds = simplecado_opt.cmds
extra_keys = fh.ExtraFitsKeywords(header_dict=hdr_dic, cmds=cmds)
opt_keys = fh.EffectsMetaKeywords(cmds=cmds)
src_keys = fh.SourceDescriptionFitsKeywords(cmds=cmds)
config_keys = fh.SimulationConfigFitsKeywords(cmds=cmds)

for eff in [src_keys, opt_keys, config_keys, extra_keys]:
simplecado_opt.optics_manager.add_effect(eff)
Expand Down
4 changes: 3 additions & 1 deletion scopesim/tests/tests_optics/test_OpticalTrain.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def simplecado_opt(mock_path_yamls):
return sim.OpticalTrain(cmd)


@pytest.mark.usefixtures("patch_mock_path")
class TestInit:
def test_initialises_with_nothing(self):
assert isinstance(OpticalTrain(), OpticalTrain)
Expand Down Expand Up @@ -294,7 +295,8 @@ def test_files_closed_on_shutdown(self, simplecado_opt, mock_path):
# Add an effect with a psf
with patch("scopesim.rc.__search_path__", [mock_path]):
psf = sim.effects.FieldConstantPSF(filename="test_ConstPSF.fits",
name="testpsf")
name="testpsf",
cmds=simplecado_opt.cmds)
simplecado_opt.optics_manager.add_effect(psf)
# This is just to make sure that we have an open file
assert not simplecado_opt['testpsf']._file._file.closed
Expand Down
2 changes: 1 addition & 1 deletion scopesim/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ def from_currsys(item, cmds=None):
if item in cmds:
item = cmds[item]
if isinstance(item, str) and item.startswith("!"):
item = from_currsys(item)
item = from_currsys(item, cmds=cmds)
else:
raise ValueError(f"{item} was not found in rc.__currsys__")

Expand Down
Loading