From b5c6b899b1c5a8101a58e662721fffd09b354df9 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:10:56 +0100 Subject: [PATCH 01/15] Add aux function for z_order --- scopesim/effects/effects_utils.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/scopesim/effects/effects_utils.py b/scopesim/effects/effects_utils.py index 58751b9f..8d11299e 100644 --- a/scopesim/effects/effects_utils.py +++ b/scopesim/effects/effects_utils.py @@ -2,6 +2,8 @@ import inspect from copy import deepcopy, copy +from collections.abc import Iterable + from astropy.table import Table from .. import effects as efs @@ -107,3 +109,28 @@ def scopesim_effect_classes(base_effect=efs.Effect): sorted_effects = {key: efs_dict[key] for key in sorted(efs_dict)} return sorted_effects + + +def z_order_in_range(z_eff, z_range: range) -> bool: + """ + Return True if any of the z_orders in `z_eff` is in the given range. + + The `z_range` parameter can be constructed as ``range(z_min, z_max)``. + + Parameters + ---------- + z_eff : int or list of ints + z_order(s) of the effect. + z_range : range + range object of allowed z_order values. + + Returns + ------- + bool + True if at least one z_order is in range, False otherwise. + + """ + if not isinstance(z_eff, Iterable): + z_eff = [z_eff] + + return any(zi in z_range for zi in z_eff) From 584ed00c9e45844e523d3b579aab8ec78b5e6b69 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:11:21 +0100 Subject: [PATCH 02/15] Refactor z_order effect gathering --- scopesim/optics/optical_element.py | 68 +++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/scopesim/optics/optical_element.py b/scopesim/optics/optical_element.py index 11f4dc5e..969c429c 100644 --- a/scopesim/optics/optical_element.py +++ b/scopesim/optics/optical_element.py @@ -6,7 +6,8 @@ from astropy.table import Table from .. import effects as efs -from ..effects.effects_utils import make_effect, get_all_effects +from ..effects.effects_utils import (make_effect, get_all_effects, + z_order_in_range) from ..utils import write_report from ..reports.rst_utils import table_to_rst from .. import rc @@ -88,27 +89,56 @@ def add_effect(self, effect): def get_all(self, effect_class): return get_all_effects(self.effects, effect_class) - def get_z_order_effects(self, z_level): - if isinstance(z_level, int): - zmin = z_level - zmax = zmin + 99 - elif isinstance(z_level, (tuple, list)): - zmin, zmax = z_level[:2] + def get_z_order_effects(self, z_level: int, z_max: int = None): + """ + Yield all effects in the given 100-range of `z_level`. + + E.g., ``z_level=200`` will yield all effect with a z_order between + 200 and 299. Optionally, the upper limit can be set manually with the + optional argument `z_max`. + + Parameters + ---------- + z_level : int + 100-range of z_orders. + z_max : int, optional + Optional upper bound. This is currently not used anywhere in + ScopeSim, but the functionality is tested. If None (default), this + will be set to ``z_level + 99``. + + Raises + ------ + TypeError + Raised if either `z_level` or `z_max` is not of int type. + ValueError + Raised if `z_max` (if given) is less than `z_level`. + + Yields + ------ + eff : Iterator of effects + Iterator containing all effect objects in the given z_order range. + + """ + if not isinstance(z_level, int): + raise TypeError(f"z_level must be int, got {type(z_level)=}") + if z_max is not None and not isinstance(z_max, int): + raise TypeError(f"If given, z_max must be int, got {type(z_max)=}") + + z_min = z_level + if z_max is not None: + if z_max < z_min: + raise ValueError( + "z_max must be greater (or equal to) z_level, but " + f"{z_max=} < {z_level=}.") else: - zmin, zmax = 0, 999 + z_max = z_min + 99 - effects = [] for eff in self.effects: - if eff.include and "z_order" in eff.meta: - z = eff.meta["z_order"] - if isinstance(z, (list, tuple)): - if any(zmin <= zi <= zmax for zi in z): - effects.append(eff) - else: - if zmin <= z <= zmax: - effects.append(eff) - - return effects + if not eff.include or "z_order" not in eff.meta: + continue + + if z_order_in_range(eff.meta["z_order"], range(z_min, z_max)): + yield eff @property def surfaces_list(self): From 969580a3207b5aa178d130200dff74b389fac690 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:11:37 +0100 Subject: [PATCH 03/15] React to changes in OpticalElement --- scopesim/optics/optics_manager.py | 5 +++-- scopesim/tests/tests_optics/test_OpticalElement.py | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 3f6b728a..669850c2 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -153,7 +153,7 @@ def get_z_order_effects(self, z_level): Parameters ---------- - z_level : int, tuple + z_level : int [0, 100, 200, 300, 400, 500] Returns @@ -163,7 +163,8 @@ def get_z_order_effects(self, z_level): """ effects = [] for opt_el in self.optical_elements: - effects += opt_el.get_z_order_effects(z_level) + # Extend evaluates generator + effects.extend(opt_el.get_z_order_effects(z_level)) return effects diff --git a/scopesim/tests/tests_optics/test_OpticalElement.py b/scopesim/tests/tests_optics/test_OpticalElement.py index b6ec10b1..73bea078 100644 --- a/scopesim/tests/tests_optics/test_OpticalElement.py +++ b/scopesim/tests/tests_optics/test_OpticalElement.py @@ -52,11 +52,13 @@ def test_currsys_ignore_effects_have_false_include_flag(self, atmo_yaml_dict): @pytest.mark.usefixtures("patch_mock_path") class TestOpticalElementGetZOrderEffects: - @pytest.mark.parametrize("z_orders, n", [(0, 2), (100, 1), ([200, 299], 1)]) - def test_returns_the_effects_with_z_values(self, z_orders, n, + @pytest.mark.parametrize("z_lvl, zmax, n", [(0, None, 2), + (100, None, 1), + (200, 299, 1)]) + def test_returns_the_effects_with_z_values(self, z_lvl, zmax, n, detector_yaml_dict): opt_el = opt_elem.OpticalElement(detector_yaml_dict) - assert len(opt_el.get_z_order_effects(z_orders)) == n + assert len(list(opt_el.get_z_order_effects(z_lvl, zmax))) == n @pytest.mark.usefixtures("patch_mock_path") From ef9d5728776b3c1fe914be747affc21eea9d6ac8 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:17:23 +0100 Subject: [PATCH 04/15] Expand docstring and typing --- scopesim/optics/optics_manager.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 669850c2..31e6779d 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -139,7 +139,7 @@ def get_all(self, class_type): return effects - def get_z_order_effects(self, z_level): + def get_z_order_effects(self, z_level: int): """ Return a list of all effects with a z_order keywords within `z_level`. @@ -150,11 +150,16 @@ def get_z_order_effects(self, z_level): - Apply Source altering effects - z_order = 200..299 - Apply FOV specific (3D) effects - z_order = 300..399 - Apply FOV-independent (2D) effects - z_order = 400..499 + - Apply XXX effects - z_order = 500..599 + - Apply XXX effects - z_order = 600..699 + - Apply lambda-independent 2D image plane effects - z_order = 700..799 + - Apply detector effects - z_order = 800..899 + - Apply detector array effects - z_order = 900..999 Parameters ---------- - z_level : int - [0, 100, 200, 300, 400, 500] + z_level : {0, 100, 200, 300, 400, 500, 600, 700, 800, 900} + 100-range of z_orders. Returns ------- From 372ac95f36f8d29552257c7e5315da5f6daf99aa Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:28:48 +0100 Subject: [PATCH 05/15] Add warning --- scopesim/effects/effects_utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scopesim/effects/effects_utils.py b/scopesim/effects/effects_utils.py index 8d11299e..dd233215 100644 --- a/scopesim/effects/effects_utils.py +++ b/scopesim/effects/effects_utils.py @@ -1,5 +1,6 @@ """TBA.""" +import logging import inspect from copy import deepcopy, copy from collections.abc import Iterable @@ -131,6 +132,7 @@ def z_order_in_range(z_eff, z_range: range) -> bool: """ if not isinstance(z_eff, Iterable): + logging.warning("z_order %d should be a single-item list", z_eff) z_eff = [z_eff] return any(zi in z_range for zi in z_eff) From 68a64d5cda7b99af5f6cb68c45b2a4505c593fea Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:35:39 +0100 Subject: [PATCH 06/15] Include sorting for effects --- scopesim/optics/optics_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 31e6779d..84fc9329 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -171,7 +171,10 @@ def get_z_order_effects(self, z_level: int): # Extend evaluates generator effects.extend(opt_el.get_z_order_effects(z_level)) - return effects + def sortkey(eff): + return next(z % 100 for z in eff.meta["z_order"]) + + return effects.sort(key=sortkey) @property def is_spectroscope(self): From 8781284212d2e781f12bc48bca43a7e492241c08 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:39:45 +0100 Subject: [PATCH 07/15] Catch earlier --- scopesim/optics/optics_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 84fc9329..8c5edd86 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -183,11 +183,10 @@ def is_spectroscope(self): @property def image_plane_headers(self): detector_lists = self.detector_setup_effects - headers = [det_list.image_plane_header for det_list in detector_lists] - if not detector_lists: raise ValueError(f"No DetectorList objects found. {detector_lists}") + headers = [det_list.image_plane_header for det_list in detector_lists] return headers @property From 326a81ccc947f6514516be353af8afcef41d1191 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 14:45:25 +0100 Subject: [PATCH 08/15] Fix for different suborders and return correctly --- scopesim/optics/optics_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 8c5edd86..1609334d 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -172,9 +172,9 @@ def get_z_order_effects(self, z_level: int): effects.extend(opt_el.get_z_order_effects(z_level)) def sortkey(eff): - return next(z % 100 for z in eff.meta["z_order"]) + return next(z % 100 for z in eff.meta["z_order"] if z >= z_level) - return effects.sort(key=sortkey) + return sorted(effects, key=sortkey) @property def is_spectroscope(self): From 3d2f212c9376c65e4f206af189e21e9400b14766 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 15:11:30 +0100 Subject: [PATCH 09/15] Simplify and add docstrings --- scopesim/optics/optics_manager.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 1609334d..555cf9f6 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -178,55 +178,62 @@ def sortkey(eff): @property def is_spectroscope(self): + """Return True if any of the effects is a spectroscope.""" return is_spectroscope(self.all_effects) @property def image_plane_headers(self): + """Get headers from detector setup effects.""" detector_lists = self.detector_setup_effects if not detector_lists: - raise ValueError(f"No DetectorList objects found. {detector_lists}") + raise ValueError("No DetectorList objects found.") - headers = [det_list.image_plane_header for det_list in detector_lists] - return headers + return [det_list.image_plane_header for det_list in detector_lists] @property def detector_array_effects(self): + """Get effects with z_order = 900...999.""" return self.get_z_order_effects(900) @property def detector_effects(self): + """Get effects with z_order = 800...899.""" return self.get_z_order_effects(800) @property def image_plane_effects(self): - effects = self.get_z_order_effects(700) - return effects + """Get effects with z_order = 700...799.""" + return self.get_z_order_effects(700) @property def fov_effects(self): - effects = self.get_z_order_effects(600) - return effects + """Get effects with z_order = 600...699.""" + return self.get_z_order_effects(600) @property def source_effects(self): + """Get effects with z_order = 500...599.""" return self.get_z_order_effects(500) # Transmission @property def detector_setup_effects(self): - # !!! Only DetectorLists go in here !!! + """Get effects with z_order = 400...499 (DetectorLists only!).""" return self.get_z_order_effects(400) @property def image_plane_setup_effects(self): + """Get effects with z_order = 300...399.""" return self.get_z_order_effects(300) @property def fov_setup_effects(self): + """Get effects with z_order = 200...299.""" # Working out where to set wave_min, wave_max return self.get_z_order_effects(200) @property def surfaces_table(self): + """Get combined surface table from effects with z_order = 100...199.""" if self._surfaces_table is None: surface_like_effects = self.get_z_order_effects(100) self._surfaces_table = combine_surface_effects(surface_like_effects) @@ -234,6 +241,7 @@ def surfaces_table(self): @property def all_effects(self): + """Get all effects in all optical elements.""" return [eff for opt_eff in self.optical_elements for eff in opt_eff] @property From 6a7632e35dbfe4d38a022b5967df956e41291a42 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Tue, 5 Dec 2023 15:18:17 +0100 Subject: [PATCH 10/15] Use generators --- scopesim/optics/optics_manager.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index 555cf9f6..d73aa4ef 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -166,15 +166,14 @@ def get_z_order_effects(self, z_level: int): effects : list of Effect objects """ - effects = [] - for opt_el in self.optical_elements: - # Extend evaluates generator - effects.extend(opt_el.get_z_order_effects(z_level)) + def _gather_effects(): + for opt_el in self.optical_elements: + yield from opt_el.get_z_order_effects(z_level) - def sortkey(eff): + def _sortkey(eff): return next(z % 100 for z in eff.meta["z_order"] if z >= z_level) - return sorted(effects, key=sortkey) + return sorted(_gather_effects(), key=_sortkey) @property def is_spectroscope(self): From 99765a55ca10b5656e5d7a7da2bf0c54b9e6fcd6 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Wed, 6 Dec 2023 12:53:16 +0100 Subject: [PATCH 11/15] More precise warning msg --- scopesim/effects/effects_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scopesim/effects/effects_utils.py b/scopesim/effects/effects_utils.py index dd233215..780c1bbc 100644 --- a/scopesim/effects/effects_utils.py +++ b/scopesim/effects/effects_utils.py @@ -132,7 +132,7 @@ def z_order_in_range(z_eff, z_range: range) -> bool: """ if not isinstance(z_eff, Iterable): - logging.warning("z_order %d should be a single-item list", z_eff) + logging.warning("z_order %d should be a single-item iterable", z_eff) z_eff = [z_eff] return any(zi in z_range for zi in z_eff) From 8682882123c471e802facf82c0114be3c6e69ee2 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Wed, 6 Dec 2023 12:53:32 +0100 Subject: [PATCH 12/15] Fix range stop argument --- scopesim/optics/optical_element.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scopesim/optics/optical_element.py b/scopesim/optics/optical_element.py index 969c429c..82b29ba7 100644 --- a/scopesim/optics/optical_element.py +++ b/scopesim/optics/optical_element.py @@ -131,7 +131,7 @@ def get_z_order_effects(self, z_level: int, z_max: int = None): "z_max must be greater (or equal to) z_level, but " f"{z_max=} < {z_level=}.") else: - z_max = z_min + 99 + z_max = z_min + 100 # range doesn't include final element -> 100 for eff in self.effects: if not eff.include or "z_order" not in eff.meta: From 26f1660c162f7cebbdfc1e4e38e8d7d64391112e Mon Sep 17 00:00:00 2001 From: teutoburg Date: Wed, 6 Dec 2023 13:02:59 +0100 Subject: [PATCH 13/15] Create range before loop --- scopesim/optics/optical_element.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scopesim/optics/optical_element.py b/scopesim/optics/optical_element.py index 82b29ba7..a9a98e61 100644 --- a/scopesim/optics/optical_element.py +++ b/scopesim/optics/optical_element.py @@ -132,12 +132,13 @@ def get_z_order_effects(self, z_level: int, z_max: int = None): f"{z_max=} < {z_level=}.") else: z_max = z_min + 100 # range doesn't include final element -> 100 + z_range = range(z_min, z_max) for eff in self.effects: if not eff.include or "z_order" not in eff.meta: continue - if z_order_in_range(eff.meta["z_order"], range(z_min, z_max)): + if z_order_in_range(eff.meta["z_order"], z_range): yield eff @property From 554a98afa82ed9b232df6ae96127b8a5f9e75ba7 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Wed, 6 Dec 2023 13:21:06 +0100 Subject: [PATCH 14/15] Minor refactoring along the way --- scopesim/optics/optical_element.py | 35 ++++++++++++++++-------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/scopesim/optics/optical_element.py b/scopesim/optics/optical_element.py index a9a98e61..f234a901 100644 --- a/scopesim/optics/optical_element.py +++ b/scopesim/optics/optical_element.py @@ -141,18 +141,18 @@ def get_z_order_effects(self, z_level: int, z_max: int = None): if z_order_in_range(eff.meta["z_order"], z_range): yield eff + def _get_matching_effects(self, effect_classes): + return (eff for eff in self.effects if isinstance(eff, effect_classes)) + @property def surfaces_list(self): - _ter_list = [effect for effect in self.effects - if isinstance(effect, (efs.SurfaceList, efs.FilterWheel, - efs.TERCurve))] - return _ter_list + effect_classes = (efs.SurfaceList, efs.FilterWheel, efs.TERCurve) + return list(self._get_matching_effects(effect_classes)) @property def masks_list(self): - _mask_list = [effect for effect in self.effects if - isinstance(effect, (efs.ApertureList, efs.ApertureMask))] - return _mask_list + effect_classes = (efs.ApertureList, efs.ApertureMask) + return list(self._get_matching_effects(effect_classes)) def list_effects(self): elements = [self.meta["name"]] * len(self.effects) @@ -254,15 +254,18 @@ def _repr_pretty_(self, p, cycle): @property def properties_str(self): - prop_str = "" - max_key_len = max(len(key) for key in self.properties.keys()) - padlen = max_key_len + 4 - for key in self.properties: - if key not in {"comments", "changes", "description", "history", - "report"}: - prop_str += f"{key:>{padlen}} : {self.properties[key]}\n" - - return prop_str + # TODO: This seems to be used only in the report below. + # Once the report uses stream writing, change this to a function + # that simply write to that same stream... + padlen = max(len(key) for key in self.properties) + 4 + exclude = {"comments", "changes", "description", "history", "report"} + + with StringIO() as str_stream: + for key in self.properties.keys() - exclude: + str_stream.write(f"{key:>{padlen}} : {self.properties[key]}\n") + output = str_stream.getvalue() + + return output def report(self, filename=None, output="rst", rst_title_chars="^#*+", **kwargs): From 4e0016cd301b820a2eee8ec1d13f366dd84e0c71 Mon Sep 17 00:00:00 2001 From: teutoburg Date: Wed, 6 Dec 2023 15:15:42 +0100 Subject: [PATCH 15/15] Don't sort effects --- scopesim/optics/optics_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scopesim/optics/optics_manager.py b/scopesim/optics/optics_manager.py index d73aa4ef..cc5242a6 100644 --- a/scopesim/optics/optics_manager.py +++ b/scopesim/optics/optics_manager.py @@ -173,7 +173,8 @@ def _gather_effects(): def _sortkey(eff): return next(z % 100 for z in eff.meta["z_order"] if z >= z_level) - return sorted(_gather_effects(), key=_sortkey) + # return sorted(_gather_effects(), key=_sortkey) + return list(_gather_effects()) @property def is_spectroscope(self):