From 84a9a2d3c01f810d63a45c64f7349720fbe97d89 Mon Sep 17 00:00:00 2001 From: oczoske Date: Tue, 9 Dec 2025 19:20:17 +0100 Subject: [PATCH 1/4] make parameter a little more general --- scopesim/effects/ter_curves.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scopesim/effects/ter_curves.py b/scopesim/effects/ter_curves.py index 166adbd7..8e613658 100644 --- a/scopesim/effects/ter_curves.py +++ b/scopesim/effects/ter_curves.py @@ -318,21 +318,21 @@ def __init__(self, cmds=None, **kwargs): remote_filename = from_currsys(kwargs["remote_filename"], cmds) kwargs["filename"] = self._download_library(remote_filename) + self.param = 'pwv' super().__init__(cmds=cmds, **kwargs) self.meta.update(kwargs) self.load_table_from_library() def update(self, **kwargs): """Update the atmo library.""" - param = 'pwv' - if param not in kwargs: - logger.warning("Can only update with parameter %s", param) + if self.param not in kwargs: + logger.warning("Can only update with parameter %s", self.param) return if "remote_filename" in kwargs: kwargs["filename"] = self._download_library(kwargs["remote_filename"]) - self.meta[param] = kwargs[param] + self.meta[self.param] = kwargs[self.param] self.load_table_from_library() @staticmethod @@ -372,6 +372,13 @@ def load_table_from_library(self): self.surface.table = tbl self.surface.meta.update(tbl.meta) + def __str__(self) -> str: + """Return str(self).""" + msg = (f"{self.__class__.__name__}: \"{self.display_name}\"\n" + f" - Parameter: {self.param} = {self.value}\n" + ) + return msg + class SkycalcTERCurve(AtmosphericTERCurve): """ From c1202eaf78b171a118f9f63850c1e7011d72ac81 Mon Sep 17 00:00:00 2001 From: oczoske Date: Mon, 29 Dec 2025 16:05:24 +0100 Subject: [PATCH 2/4] compliment -> complement to prevent mental anguish --- scopesim/optics/surface.py | 34 +++++++++---------- .../tests_optics/test_SpectralSurface.py | 10 +++--- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/scopesim/optics/surface.py b/scopesim/optics/surface.py index 9ba1e04b..7665e506 100644 --- a/scopesim/optics/surface.py +++ b/scopesim/optics/surface.py @@ -185,14 +185,14 @@ def _get_ter_property(self, ter_property, fmt="synphot"): response_curve : ``synphot.SpectralElement`` """ - compliment_names = ["transmission", "emissivity", "reflection"] - ii = np.where([ter_property == name for name in compliment_names])[0][0] - compliment_names.pop(ii) + complement_names = ["transmission", "emissivity", "reflection"] + ii = np.where([ter_property == name for name in complement_names])[0][0] + complement_names.pop(ii) wave = self._get_array("wavelength") value_arr = self._get_array(ter_property) if value_arr is None: - value_arr = self._compliment_array(*compliment_names) + value_arr = self._complement_array(*complement_names) if value_arr is not None and wave is not None and fmt == "synphot": response_curve = SpectralElement(Empirical1D, points=wave, @@ -205,9 +205,9 @@ def _get_ter_property(self, ter_property, fmt="synphot"): return response_curve - def _compliment_array(self, colname_a, colname_b): + def _complement_array(self, colname_a, colname_b): """ - Return an complimentary array using: ``a + b + c = 1``. + Return an complementary array using: ``a + b + c = 1``. E.g. ``Emissivity = 1 - (Transmission + Reflection)`` @@ -221,20 +221,18 @@ def _compliment_array(self, colname_a, colname_b): Returns ------- actual : ``synphot.SpectralElement`` - Complimentary spectrum to those given + Complementary spectrum to those given """ - compliment_a = self._get_array(colname_a) - compliment_b = self._get_array(colname_b) - - if compliment_a is not None and compliment_b is not None: - actual = 1 * compliment_a.unit - (compliment_a + compliment_b) - elif compliment_a is not None and compliment_b is None: - actual = 1 * compliment_a.unit - compliment_a - elif compliment_b is not None and compliment_a is None: - actual = 1 * compliment_b.unit - compliment_b - elif compliment_b is None and compliment_a is None: - actual = None + complement_a = self._get_array(colname_a) + complement_b = self._get_array(colname_b) + actual = None + if complement_a is not None and complement_b is not None: + actual = 1 * complement_a.unit - (complement_a + complement_b) + elif complement_a is not None and complement_b is None: + actual = 1 * complement_a.unit - complement_a + elif complement_b is not None and complement_a is None: + actual = 1 * complement_b.unit - complement_b return actual diff --git a/scopesim/tests/tests_optics/test_SpectralSurface.py b/scopesim/tests/tests_optics/test_SpectralSurface.py index df0a325f..6f572378 100644 --- a/scopesim/tests/tests_optics/test_SpectralSurface.py +++ b/scopesim/tests/tests_optics/test_SpectralSurface.py @@ -3,7 +3,7 @@ # pylint: disable=missing-function-docstring # 1 read in a table -# 2 compliment the table based on columns in file +# 2 complement the table based on columns in file # 3 have @property methods for: transmission, ermission, reflection import pytest @@ -142,7 +142,7 @@ def test_returned_bb_curve_is_scaled_to_per_arcsec2(self): srf.emission(wave)) -class TestSpectralSurfaceComplimentArray: +class TestSpectralSurfaceComplementArray: @pytest.mark.parametrize("colname1, colname2, col1, col2, expected", [("A", "B", [0.8]*u.um, [0.1]*u.um, [0.1]*u.um), ("A", "B", [0.8]*u.um, None, [0.2]*u.um), @@ -152,7 +152,7 @@ def test_the_right_answers_for_valid_input(self, colname1, colname2, srf = opt_surf.SpectralSurface() srf.meta[colname1] = col1 srf.meta[colname2] = col2 - col3 = srf._compliment_array(colname1, colname2) + col3 = srf._complement_array(colname1, colname2) assert np.allclose(col3.data, expected.data) assert col3.unit == expected.unit @@ -163,7 +163,7 @@ def test_returns_none_for_none_input(self, colname1, colname2, srf = opt_surf.SpectralSurface() srf.meta[colname1] = col1 srf.meta[colname2] = col2 - col3 = srf._compliment_array(colname1, colname2) + col3 = srf._complement_array(colname1, colname2) assert col3 is None @pytest.mark.parametrize("col2_arr, expected", @@ -177,7 +177,7 @@ def test_returns_right_answers_for_valid_table(self, col2_arr, expected): if col2_arr: srf.table.add_column(Column(name="col2", data=col2_arr)) - col3 = srf._compliment_array("col1", "col2") + col3 = srf._complement_array("col1", "col2") assert col3.data == pytest.approx(expected) assert len(col3.data) == len(expected) From a3565e43744a00fb2a817095dd0c3a55ffe64d39 Mon Sep 17 00:00:00 2001 From: oczoske Date: Mon, 29 Dec 2025 17:00:15 +0100 Subject: [PATCH 3/4] Make SkycalcTERCurve updatable --- scopesim/effects/ter_curves.py | 33 +++++++++++++++-- scopesim/source/source_fields.py | 2 +- .../tests_effects/test_SkycalcTERCurve.py | 36 ++++++++++++++++++- 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/scopesim/effects/ter_curves.py b/scopesim/effects/ter_curves.py index 8e613658..137a77ca 100644 --- a/scopesim/effects/ter_curves.py +++ b/scopesim/effects/ter_curves.py @@ -420,6 +420,11 @@ def __init__(self, **kwargs): self.skycalc_table = None self.skycalc_conn = None + use_local_file = from_currsys(self.meta["use_local_skycalc_file"], + self.cmds) + if not use_local_file: + self.skycalc_conn = skycalc_ipy.SkyCalc() + if self.include is True: # Only query the database if the effect is actually included. # Sets skycalc_conn and skycalc_table. @@ -436,10 +441,10 @@ def include(self, item): self.load_skycalc_table() def load_skycalc_table(self): + """Download skycalc table based on the current parameters""" use_local_file = from_currsys(self.meta["use_local_skycalc_file"], self.cmds) if not use_local_file: - self.skycalc_conn = skycalc_ipy.SkyCalc() tbl = self.query_server() if "name" not in self.meta: @@ -469,6 +474,7 @@ def load_skycalc_table(self): self.skycalc_table = tbl def query_server(self, **kwargs): + """Get table from the skycalc server""" self.meta.update(kwargs) if "wunit" in self.meta: @@ -478,6 +484,7 @@ def query_server(self, **kwargs): if key in self.meta: self.meta[key] = from_currsys(self.meta[key], self.cmds) * scale_factor + self.meta["wunit"] = "nm" conn_kwargs = {key: self.meta[key] for key in self.meta if key in self.skycalc_conn.defaults} @@ -486,13 +493,33 @@ def query_server(self, **kwargs): try: tbl = self.skycalc_conn.get_sky_spectrum(return_type="table") - except ConnectionError: + except ConnectionError as exc: msg = "Could not connect to skycalc server" logger.exception(msg) - raise ValueError(msg) + raise ValueError(msg) from exc return tbl + def update(self, **kwargs): + """Update the skycalc table with new parameter values""" + self._background_source = None + # Validate parameters + for key in kwargs: + if key not in self.skycalc_conn.keys: + logger.warning("Parameter %s not recognised. Ignored." % key) + kwargs.pop(key, None) + self.meta.update(kwargs) + self.load_skycalc_table() + + def __str__(self): + """Return str(self).""" + msg = f"{self.__class__.__name__}: \"{self.display_name}\"\n" + for key, val in self.skycalc_conn.values.items(): + comment = self.skycalc_conn.comments[key] + msg += f"\x1b[32m{'# ' + comment}'\x1b[0m\n" + msg += f"{key:15s}: {str(val):24s}\n" + return msg + class QuantumEfficiencyCurve(TERCurve): z_order: ClassVar[tuple[int, ...]] = (113, 513) diff --git a/scopesim/source/source_fields.py b/scopesim/source/source_fields.py index fe685ab7..2745e9e9 100644 --- a/scopesim/source/source_fields.py +++ b/scopesim/source/source_fields.py @@ -449,4 +449,4 @@ def get_corners(self, unit: u.Unit | str = "arcsec") -> np.ndarray: return np.array([-np.inf, np.inf]) def _write_stream(self, stream: TextIO) -> None: - stream.write(f"Background field ref. spectrum {self.spectrum[0]}") + stream.write(f"Background field ref. spectrum {self.spectrum}") diff --git a/scopesim/tests/tests_effects/test_SkycalcTERCurve.py b/scopesim/tests/tests_effects/test_SkycalcTERCurve.py index 41b77430..689621b1 100644 --- a/scopesim/tests/tests_effects/test_SkycalcTERCurve.py +++ b/scopesim/tests/tests_effects/test_SkycalcTERCurve.py @@ -1,7 +1,13 @@ +"""Unit tests for SkycalcTERCurve""" +# pylint: disable=missing-function-docstring +# pylint: disable=invalid-name +# pylint: disable=too-few-public-methods from pathlib import Path -import pytest from unittest.mock import patch +import pytest + +from astropy import units as u from synphot import SpectralElement, SourceSpectrum from scopesim.effects import SkycalcTERCurve @@ -22,6 +28,7 @@ def setup_and_teardown(): @pytest.mark.filterwarnings("ignore::astropy.units.core.UnitsWarning") class TestInit: + """Test initialisation of the effect""" @pytest.mark.webtest def test_initialises_with_nothing(self): assert isinstance(SkycalcTERCurve(), SkycalcTERCurve) @@ -54,3 +61,30 @@ def test_initialise_with_local_skycalc_file(self, mock_path): sky_ter = SkycalcTERCurve( use_local_skycalc_file=str(mock_path / "skycalc_override.fits")) assert sky_ter.skycalc_table is not None + + +class TestUpdate: + """Test updating of the effect""" + @pytest.mark.webtest + def test_changes_meta_parameter(self): + sky_ter = SkycalcTERCurve(observatory="paranal") + assert sky_ter.meta["observatory"] == "paranal" + sky_ter.update(observatory="armazones") + assert sky_ter.meta["observatory"] == "armazones" + assert sky_ter.skycalc_conn.values["observatory"] == "armazones" + + @pytest.mark.webtest + def test_increasing_pwv_increases_emission(self): + sky_ter = SkycalcTERCurve(pwv=1.0) + val1 = sky_ter.surface.emission(10 * u.um) + sky_ter.update(pwv=20.) + val2 = sky_ter.surface.emission(10 * u.um) + assert val2 > val1 + + @pytest.mark.webtest + def test_increasing_pwv_decreases_transmission(self): + sky_ter = SkycalcTERCurve(pwv=1.0) + val1 = sky_ter.surface.throughput(10 * u.um) + sky_ter.update(pwv=20.) + val2 = sky_ter.surface.throughput(10 * u.um) + assert val2 < val1 From c16101171087efaf8b0f64a3531008bd1272f020 Mon Sep 17 00:00:00 2001 From: oczoske Date: Mon, 29 Dec 2025 17:24:53 +0100 Subject: [PATCH 4/4] Just an idea, but not yet practicable --- scopesim/effects/ter_curves.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scopesim/effects/ter_curves.py b/scopesim/effects/ter_curves.py index 137a77ca..32d85319 100644 --- a/scopesim/effects/ter_curves.py +++ b/scopesim/effects/ter_curves.py @@ -440,6 +440,14 @@ def include(self, item): if item is True and self.skycalc_table is None: self.load_skycalc_table() + ## Nice to have, but would need a setter + ## self.parameters['pwv'] = 20. + ## This would have to set self.meta['pwv'] at the same time, + ## otherwise meta would always override. + #@property + #def parameters(self): + # return self.skycalc_conn.values + def load_skycalc_table(self): """Download skycalc table based on the current parameters""" use_local_file = from_currsys(self.meta["use_local_skycalc_file"], @@ -502,7 +510,9 @@ def query_server(self, **kwargs): def update(self, **kwargs): """Update the skycalc table with new parameter values""" + # Needed to update the source field self._background_source = None + # Validate parameters for key in kwargs: if key not in self.skycalc_conn.keys: