From 0ed7e450ba216db05c58e5027a7feccbe7a4d861 Mon Sep 17 00:00:00 2001 From: Martin Raspaud Date: Tue, 3 Mar 2026 12:01:21 +0100 Subject: [PATCH 1/4] Introduce composite tags --- doc/source/composites.rst | 158 ++++++++++++++++++ doc/source/config.rst | 40 +++++ satpy/dependency_tree.py | 83 ++++++++- .../compositor_tests/test_config_loader.py | 72 ++++++++ satpy/tests/test_dependency_tree.py | 88 ++++++++++ 5 files changed, 434 insertions(+), 7 deletions(-) diff --git a/doc/source/composites.rst b/doc/source/composites.rst index 520ec626a4..5be6ff4f72 100644 --- a/doc/source/composites.rst +++ b/doc/source/composites.rst @@ -495,6 +495,164 @@ image) for both of the static images:: min_stretch: [0, 0, 0] max_stretch: [255, 255, 255] +.. _composite_variants: + +Composite variants +------------------ + +.. versionadded:: 0.60 + +Satpy supports defining multiple *variants* of a composite (e.g., one that +applies WMO-recommended recipe and one that does not). This feature is controlled by optional +fields in the composite YAML configuration. + +Tagging composite variants +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +A composite can carry a list of **tags** that describe which processing variant +it represents. Tags are plain strings (e.g., ``wmo``, ``crefl``, +``nocorr``) and have no special meaning to Satpy beyond being matched during +compositor lookup. + +.. code-block:: yaml + + sensor_name: visir + + composites: + true_color_wmo: + compositor: !!python/name:satpy.composites.SomeCompositor + prerequisites: + - name: red + modifiers: [rayleigh_corrected_wmo] + - name: green + - name: blue + standard_name: true_color + tags: [wmo] + + true_color_crefl: + compositor: !!python/name:satpy.composites.SomeCompositor + prerequisites: + - name: red + modifiers: [rayleigh_corrected_crefl] + - name: green + - name: blue + standard_name: true_color + tags: [crefl] + + true_color: # default, no tags + compositor: !!python/name:satpy.composites.SomeCompositor + prerequisites: + - name: red + - name: green + - name: blue + standard_name: true_color + +The ``standard_name`` field acts as the *base name* shared by all variants. +Tag-based resolution searches for a compositor whose ``standard_name`` matches +and whose ``tags`` list contains the requested tag. + +Loading a tagged variant explicitly +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Append ``:`` to the composite name when calling :meth:`~satpy.scene.Scene.load`: + +.. code-block:: python + + scene.load(["true_color:wmo"]) # loads true_color_wmo + +This syntax is interpreted as: "find a compositor with +``standard_name='true_color'`` that has ``'wmo'`` in its ``tags``". It never +performs a plain string match, so ``true_color:wmo`` will not accidentally +resolve to a compositor named ``true_color`` or ``true_color_wmo``. + +Multiple tags can be combined with additional colons. All listed tags must be +present on the compositor (AND semantics): + +.. code-block:: python + + scene.load(["true_color:wmo:pyspectral"]) # compositor must carry wmo AND pyspectral + scene.load(["true_color:wmo"]) # also matches a compositor with tags [wmo, pyspectral] + +A single-tag request is a subset match, so requesting ``"true_color:wmo"`` +will find a compositor tagged ``["wmo", "pyspectral"]`` just as readily as one +tagged ``["wmo"]`` alone. + +Session-wide tag preferences +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +You can configure an **ordered** list of preferred tags so that loading an +unqualified name like ``"true_color"`` automatically selects a tagged variant +when one is available: + +.. code-block:: python + + import satpy + with satpy.config.set(preferred_composite_tags=["crefl", "wmo"]): + scene.load(["true_color"]) # picks true_color_crefl (crefl listed first) + +Resolution order for ``preferred_composite_tags``: + +1. Try each tag in the list, in order, looking for a compositor with matching + ``standard_name`` and that tag. +2. If no tagged variant is found, fall back to the normal name-based lookup. + +An explicit ``name:tag`` in the load call always overrides the session-wide +preference for that specific dataset. + +The setting can also be provided as an environment variable (comma-separated): + +.. code-block:: bash + + export SATPY_PREFERRED_COMPOSITE_TAGS=crefl,wmo + +Or as a YAML configuration key: + +.. code-block:: yaml + + preferred_composite_tags: + - crefl + - wmo + +Enhancements for tagged variants +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +No extra configuration is needed on the enhancement side. The enhancement +decision tree already matches by composite ``name`` (the YAML key) *before* +it falls back to ``standard_name``. A composite named ``true_color_wmo`` will +therefore automatically pick up an enhancement entry keyed ``true_color_wmo`` +if one exists, and fall back to the ``standard_name: true_color`` entry +otherwise. + +Deprecating a composite +------------------------ + +To emit a warning when a composite is used, add a ``warnings`` mapping to the +composite YAML entry. Each key is a Python warning category name and the +value is the warning message: + +.. code-block:: yaml + + composites: + old_true_color: + compositor: !!python/name:satpy.composites.SomeCompositor + prerequisites: + - name: red + - name: green + - name: blue + standard_name: true_color + warnings: + DeprecationWarning: "old_true_color is deprecated, use true_color_wmo instead." + +The warning is emitted only when the compositor is actually *loaded* (i.e. +when :meth:`~satpy.scene.Scene.load` is called with the deprecated name), not +during composite discovery calls such as +:meth:`~satpy.scene.Scene.available_dataset_names`. + +Supported warning categories are any that exist in the Python ``builtins`` +module (e.g., ``DeprecationWarning``, ``FutureWarning``, +``PendingDeprecationWarning``, ``UserWarning``). If a category name is not +recognised, ``UserWarning`` is used as a fallback. + .. _enhancing-the-images: Enhancing the images diff --git a/doc/source/config.rst b/doc/source/config.rst index 2b66a83ebb..0d21a9c6bb 100644 --- a/doc/source/config.rst +++ b/doc/source/config.rst @@ -279,6 +279,46 @@ Clipping of negative radiances is currently implemented for the following reader * ``abi_l1b``, ``ami_l1b``, ``fci_l1c_nc`` +Preferred Composite Tags +^^^^^^^^^^^^^^^^^^^^^^^^ + +* **Environment variable**: ``SATPY_PREFERRED_COMPOSITE_TAGS`` +* **YAML/Config Key**: ``preferred_composite_tags`` +* **Default**: ``[]`` + +Ordered list of composite variant tags that Satpy should prefer when resolving +an unqualified composite name. When a user requests a composite such as +``"true_color"`` and this list is non-empty, Satpy will first search for a +compositor whose ``standard_name`` matches and whose ``tags`` list contains +the first tag in the preference list, then the second tag, and so on. If no +tagged variant is found the normal name-based lookup is used as a fallback. + +For example, to prefer Pyspectral-based variants: + +.. code-block:: python + + import satpy + satpy.config.set(preferred_composite_tags=["pyspectral"]) + +Or to prefer CREFL Rayleigh correction over Pyspectral: + +.. code-block:: python + + satpy.config.set(preferred_composite_tags=["crefl", "pyspectral"]) + +An explicit ``name:tag`` syntax in the ``scene.load()`` call always overrides +this setting for that specific dataset. + +When setting this as an environment variable, it should be a comma-separated +list of tag names, for example: + +.. code-block:: bash + + export SATPY_PREFERRED_COMPOSITE_TAGS=crefl,pyspectral + +See :ref:`composite_variants` for a full description of +composite tagging and the ``name:tag`` load syntax. + Temporary Directory ^^^^^^^^^^^^^^^^^^^ diff --git a/satpy/dependency_tree.py b/satpy/dependency_tree.py index 97777a71e9..8fec4dad66 100644 --- a/satpy/dependency_tree.py +++ b/satpy/dependency_tree.py @@ -19,10 +19,13 @@ from __future__ import annotations +import builtins +import warnings from typing import Container, Iterable, Optional import numpy as np +import satpy from satpy import DataID, DatasetDict from satpy.dataset import ModifierTuple, create_filtered_query from satpy.dataset.data_dict import TooManyResults, get_key @@ -492,14 +495,80 @@ def _promote_query_to_modified_dataid(self, query, dep_key): return dep_key.from_dict(orig_dict) def get_compositor(self, key): - """Get a compositor.""" - for sensor_name in sorted(self.compositors): - try: - return self.compositors[sensor_name][key] - except KeyError: - continue + """Get a compositor. + + Resolves in order: + 1. Tag-based: explicit ``name:tag`` syntax or ``preferred_composite_tags`` config. + 2. Normal name-based lookup in the compositor registry. + + If the compositor has a ``warnings`` attribute dict, those warnings are emitted here. + """ + compositor = self._get_compositor_by_tag(key) + if compositor is None: + for sensor_name in sorted(self.compositors): + try: + compositor = self.compositors[sensor_name][key] + break + except KeyError: + continue + + if compositor is None: + raise KeyError("Could not find compositor '{}'".format(key)) - raise KeyError("Could not find compositor '{}'".format(key)) + self._emit_compositor_warnings(compositor) + return compositor + + @staticmethod + def _emit_compositor_warnings(compositor): + for category_name, message in compositor.attrs.get("warnings", {}).items(): + category = getattr(builtins, category_name, UserWarning) + warnings.warn(message, category, stacklevel=4) + + def _get_compositor_by_tag(self, key): + """Find a compositor by tag syntax (``'name:tag1:tag2'``) or ``preferred_composite_tags`` config. + + For explicit tag syntax the returned compositor must carry all listed tags; among + multiple matches the ``preferred_composite_tags`` config is used as a tiebreaker, + falling back to the first candidate in alphabetical sensor order. + + For plain names (no colon) each entry in ``preferred_composite_tags`` is tried in + order; ``None`` is returned when none match so that normal name-based lookup can + proceed. + """ + try: + name = key["name"] + except (KeyError, TypeError): + return None + if not isinstance(name, str): + return None + + parts = name.split(":") + standard_name, required_tags = parts[0], set(parts[1:]) + candidates = self._find_tag_candidates(standard_name, required_tags) + preferred = self._pick_preferred_candidate(candidates) + if preferred is not None: + return preferred + # Explicit-tag requests fall back to the first candidate; plain-name requests + # return None so that normal name-based lookup can proceed. + return candidates[0] if required_tags else None + + def _find_tag_candidates(self, standard_name, required_tags): + """Return compositors whose standard_name matches and that carry all required_tags.""" + return [ + comp + for sensor_name in sorted(self.compositors) + for comp in self.compositors[sensor_name].values() + if comp.attrs.get("standard_name") == standard_name + and required_tags.issubset(set(comp.attrs.get("tags", []))) + ] + + def _pick_preferred_candidate(self, candidates): + """Return the first candidate that matches any preferred_composite_tags entry, in order.""" + for tag in satpy.config.get("preferred_composite_tags", []): + for comp in candidates: + if tag in comp.attrs.get("tags", []): + return comp + return None def get_modifier(self, comp_id): """Get a modifer.""" diff --git a/satpy/tests/compositor_tests/test_config_loader.py b/satpy/tests/compositor_tests/test_config_loader.py index 6ddedcb641..2242cc6201 100644 --- a/satpy/tests/compositor_tests/test_config_loader.py +++ b/satpy/tests/compositor_tests/test_config_loader.py @@ -56,3 +56,75 @@ def _create_fake_composite_config(yaml_filename: str): }, comp_file, ) + + +def test_composite_warning_is_emitted_on_compositor_use(tmp_path): + """Test that composite 'warnings' fires when the compositor is fetched, not when configs are loaded.""" + import pytest + + from satpy.composites.config_loader import load_compositor_configs_for_sensors + from satpy.dataset import DataQuery + from satpy.dependency_tree import DependencyTree + + comp_dir = tmp_path / "composites" + comp_dir.mkdir() + _create_fake_composite_config_with_warnings(comp_dir / "fake_sensor.yaml") + + with satpy.config.set(config_path=[tmp_path]): + comps, _ = load_compositor_configs_for_sensors(["fake_sensor"]) # no warning here + + tree = DependencyTree({}, comps, {}) + with pytest.warns(DeprecationWarning, match="Use new_composite instead"): + tree.get_compositor(DataQuery(name="old_composite")) + + +def _create_fake_composite_config_with_warnings(yaml_filename): + import yaml + + from satpy.composites.aux_data import StaticImageCompositor + + with open(yaml_filename, "w") as comp_file: + yaml.dump({ + "sensor_name": "fake_sensor", + "composites": { + "old_composite": { + "compositor": StaticImageCompositor, + "url": "http://example.com/image.png", + "warnings": {"DeprecationWarning": "Use new_composite instead"}, + }, + }, + }, comp_file) + + +def test_composite_tags_stored_on_compositor(tmp_path): + """Test that a composite with 'tags' in its YAML has those tags stored in its attrs.""" + from satpy.composites.config_loader import load_compositor_configs_for_sensors + + comp_dir = tmp_path / "composites" + comp_dir.mkdir() + comp_yaml = comp_dir / "fake_sensor.yaml" + _create_fake_composite_config_with_tags(comp_yaml, tags=["wmo"]) + + with satpy.config.set(config_path=[tmp_path]): + comps, _ = load_compositor_configs_for_sensors(["fake_sensor"]) + + compositor = next(iter(comps["fake_sensor"].values())) + assert compositor.attrs.get("tags") == ["wmo"] + + +def _create_fake_composite_config_with_tags(yaml_filename, tags): + import yaml + + from satpy.composites.aux_data import StaticImageCompositor + + with open(yaml_filename, "w") as comp_file: + yaml.dump({ + "sensor_name": "fake_sensor", + "composites": { + "tagged_composite": { + "compositor": StaticImageCompositor, + "url": "http://example.com/image.png", + "tags": tags, + }, + }, + }, comp_file) diff --git a/satpy/tests/test_dependency_tree.py b/satpy/tests/test_dependency_tree.py index 1082d983ee..e4a7abe6a4 100644 --- a/satpy/tests/test_dependency_tree.py +++ b/satpy/tests/test_dependency_tree.py @@ -96,6 +96,94 @@ def test_new_dependency_tree_preserves_unique_empty_node(self): assert self.dependency_tree.empty_node is new_dependency_tree.empty_node +class TestGetCompositorByTag: + """Test tag-based compositor resolution via 'name:tag' syntax.""" + + def test_get_compositor_finds_tagged_variant(self): + """Test that 'name:tag' in get_compositor finds a compositor with matching standard_name and tag.""" + from satpy.dataset import DataQuery, DatasetDict + from satpy.tests.utils import FakeCompositor, make_cid + + comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], standard_name="comp1", tags=["wmo"]) + compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo"): comp_wmo})} + + tree = DependencyTree({}, compositors, {}) + result = tree.get_compositor(DataQuery(name="comp1:wmo")) + assert result is comp_wmo + + + def test_get_compositor_finds_multi_tag_variant(self): + """Test that 'name:tag1:tag2' finds a compositor carrying all listed tags.""" + from satpy.dataset import DataQuery, DatasetDict + from satpy.tests.utils import FakeCompositor, make_cid + + comp = FakeCompositor(name="comp1_wmo_pyspectral", prerequisites=[], + standard_name="comp1", tags=["wmo", "pyspectral"]) + compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo_pyspectral"): comp})} + + tree = DependencyTree({}, compositors, {}) + result = tree.get_compositor(DataQuery(name="comp1:wmo:pyspectral")) + assert result is comp + + def test_single_tag_request_matches_multi_tag_compositor(self): + """Test that 'name:tag' matches a compositor that carries that tag plus others.""" + from satpy.dataset import DataQuery, DatasetDict + from satpy.tests.utils import FakeCompositor, make_cid + + comp = FakeCompositor(name="comp1_wmo_pyspectral", prerequisites=[], + standard_name="comp1", tags=["wmo", "pyspectral"]) + compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo_pyspectral"): comp})} + + tree = DependencyTree({}, compositors, {}) + result = tree.get_compositor(DataQuery(name="comp1:wmo")) + assert result is comp + + def test_explicit_tag_respects_preferred_tags_as_tiebreaker(self): + """Test that preferred_composite_tags breaks ties among compositors all matching an explicit tag. + + When 'true_color:wmo' is requested and both true_color_wmo (tags=[wmo]) and + true_color_wmo_pyspectral (tags=[wmo, pyspectral]) are available, the compositor + whose extra tags include the preferred tag should win. + """ + import satpy + from satpy.dataset import DataQuery, DatasetDict + from satpy.tests.utils import FakeCompositor, make_cid + + comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], + standard_name="comp1", tags=["wmo"]) + comp_wmo_pyspectral = FakeCompositor(name="comp1_wmo_pyspectral", prerequisites=[], + standard_name="comp1", tags=["wmo", "pyspectral"]) + compositors = {"fake_sensor": DatasetDict({ + make_cid(name="comp1_wmo"): comp_wmo, + make_cid(name="comp1_wmo_pyspectral"): comp_wmo_pyspectral, + })} + tree = DependencyTree({}, compositors, {}) + + with satpy.config.set(preferred_composite_tags=["pyspectral"]): + result = tree.get_compositor(DataQuery(name="comp1:wmo")) + + assert result is comp_wmo_pyspectral + + def test_preferred_tags_selects_first_matching_compositor(self): + """Test that preferred_composite_tags config selects the first matching tagged compositor.""" + import satpy + from satpy.dataset import DataQuery, DatasetDict + from satpy.tests.utils import FakeCompositor, make_cid + + comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], standard_name="comp1", tags=["wmo"]) + comp_crefl = FakeCompositor(name="comp1_crefl", prerequisites=[], standard_name="comp1", tags=["crefl"]) + compositors = {"fake_sensor": DatasetDict({ + make_cid(name="comp1_wmo"): comp_wmo, + make_cid(name="comp1_crefl"): comp_crefl, + })} + tree = DependencyTree({}, compositors, {}) + + with satpy.config.set(preferred_composite_tags=["crefl", "wmo"]): + result = tree.get_compositor(DataQuery(name="comp1")) + + assert result is comp_crefl + + class TestMissingDependencies(unittest.TestCase): """Test the MissingDependencies exception.""" From 94a835ea45420a379b340ba6f2c6abacec64ccdc Mon Sep 17 00:00:00 2001 From: Martin Raspaud Date: Tue, 3 Mar 2026 12:24:42 +0100 Subject: [PATCH 2/4] Parametrise tests --- satpy/tests/test_dependency_tree.py | 134 +++++++++++----------------- 1 file changed, 53 insertions(+), 81 deletions(-) diff --git a/satpy/tests/test_dependency_tree.py b/satpy/tests/test_dependency_tree.py index e4a7abe6a4..50d6477cec 100644 --- a/satpy/tests/test_dependency_tree.py +++ b/satpy/tests/test_dependency_tree.py @@ -18,8 +18,12 @@ import os import unittest +import pytest + +import satpy +from satpy.dataset import DataQuery, DatasetDict from satpy.dependency_tree import DependencyTree -from satpy.tests.utils import make_cid, make_dataid +from satpy.tests.utils import FakeCompositor, make_cid, make_dataid class TestDependencyTree(unittest.TestCase): @@ -99,89 +103,57 @@ def test_new_dependency_tree_preserves_unique_empty_node(self): class TestGetCompositorByTag: """Test tag-based compositor resolution via 'name:tag' syntax.""" - def test_get_compositor_finds_tagged_variant(self): - """Test that 'name:tag' in get_compositor finds a compositor with matching standard_name and tag.""" - from satpy.dataset import DataQuery, DatasetDict - from satpy.tests.utils import FakeCompositor, make_cid - - comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], standard_name="comp1", tags=["wmo"]) - compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo"): comp_wmo})} - - tree = DependencyTree({}, compositors, {}) - result = tree.get_compositor(DataQuery(name="comp1:wmo")) - assert result is comp_wmo - - - def test_get_compositor_finds_multi_tag_variant(self): - """Test that 'name:tag1:tag2' finds a compositor carrying all listed tags.""" - from satpy.dataset import DataQuery, DatasetDict - from satpy.tests.utils import FakeCompositor, make_cid - - comp = FakeCompositor(name="comp1_wmo_pyspectral", prerequisites=[], - standard_name="comp1", tags=["wmo", "pyspectral"]) - compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo_pyspectral"): comp})} - - tree = DependencyTree({}, compositors, {}) - result = tree.get_compositor(DataQuery(name="comp1:wmo:pyspectral")) - assert result is comp - - def test_single_tag_request_matches_multi_tag_compositor(self): - """Test that 'name:tag' matches a compositor that carries that tag plus others.""" - from satpy.dataset import DataQuery, DatasetDict - from satpy.tests.utils import FakeCompositor, make_cid - - comp = FakeCompositor(name="comp1_wmo_pyspectral", prerequisites=[], - standard_name="comp1", tags=["wmo", "pyspectral"]) - compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo_pyspectral"): comp})} - - tree = DependencyTree({}, compositors, {}) - result = tree.get_compositor(DataQuery(name="comp1:wmo")) - assert result is comp - - def test_explicit_tag_respects_preferred_tags_as_tiebreaker(self): - """Test that preferred_composite_tags breaks ties among compositors all matching an explicit tag. - - When 'true_color:wmo' is requested and both true_color_wmo (tags=[wmo]) and - true_color_wmo_pyspectral (tags=[wmo, pyspectral]) are available, the compositor - whose extra tags include the preferred tag should win. - """ - import satpy - from satpy.dataset import DataQuery, DatasetDict - from satpy.tests.utils import FakeCompositor, make_cid - - comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], - standard_name="comp1", tags=["wmo"]) - comp_wmo_pyspectral = FakeCompositor(name="comp1_wmo_pyspectral", prerequisites=[], - standard_name="comp1", tags=["wmo", "pyspectral"]) - compositors = {"fake_sensor": DatasetDict({ - make_cid(name="comp1_wmo"): comp_wmo, - make_cid(name="comp1_wmo_pyspectral"): comp_wmo_pyspectral, - })} - tree = DependencyTree({}, compositors, {}) - - with satpy.config.set(preferred_composite_tags=["pyspectral"]): - result = tree.get_compositor(DataQuery(name="comp1:wmo")) - - assert result is comp_wmo_pyspectral - - def test_preferred_tags_selects_first_matching_compositor(self): - """Test that preferred_composite_tags config selects the first matching tagged compositor.""" - import satpy - from satpy.dataset import DataQuery, DatasetDict - from satpy.tests.utils import FakeCompositor, make_cid - - comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], standard_name="comp1", tags=["wmo"]) - comp_crefl = FakeCompositor(name="comp1_crefl", prerequisites=[], standard_name="comp1", tags=["crefl"]) - compositors = {"fake_sensor": DatasetDict({ - make_cid(name="comp1_wmo"): comp_wmo, - make_cid(name="comp1_crefl"): comp_crefl, - })} + @pytest.mark.parametrize(("compositors_spec", "preferred_tags", "query", "expected_name"), [ + pytest.param( + {"comp1_wmo": ["wmo"]}, + [], + "comp1:wmo", + "comp1_wmo", + id="single_explicit_tag", + ), + pytest.param( + {"comp1_wmo_pyspectral": ["wmo", "pyspectral"]}, + [], + "comp1:wmo:pyspectral", + "comp1_wmo_pyspectral", + id="multi_explicit_tags", + ), + pytest.param( + {"comp1_wmo_pyspectral": ["wmo", "pyspectral"]}, + [], + "comp1:wmo", + "comp1_wmo_pyspectral", + id="single_tag_matches_multi_tag_compositor", + ), + pytest.param( + {"comp1_wmo": ["wmo"], "comp1_wmo_pyspectral": ["wmo", "pyspectral"]}, + ["pyspectral"], + "comp1:wmo", + "comp1_wmo_pyspectral", + id="preferred_tags_tiebreaker_for_explicit_tag", + ), + pytest.param( + {"comp1_wmo": ["wmo"], "comp1_crefl": ["crefl"]}, + ["crefl", "wmo"], + "comp1", + "comp1_crefl", + id="preferred_tags_for_plain_name", + ), + ]) + def test_get_compositor_by_tag(self, compositors_spec, preferred_tags, query, expected_name): + """Test compositor resolution using tag syntax and preferred_composite_tags config.""" + compositors = { + "fake_sensor": DatasetDict({ + make_cid(name=name): FakeCompositor(name=name, prerequisites=[], standard_name="comp1", tags=tags) + for name, tags in compositors_spec.items() + }) + } tree = DependencyTree({}, compositors, {}) - with satpy.config.set(preferred_composite_tags=["crefl", "wmo"]): - result = tree.get_compositor(DataQuery(name="comp1")) + with satpy.config.set(preferred_composite_tags=preferred_tags): + result = tree.get_compositor(DataQuery(name=query)) - assert result is comp_crefl + assert result.attrs["name"] == expected_name class TestMissingDependencies(unittest.TestCase): From 7557662fe9d53f3deaa05ce2ec35a96da80fff13 Mon Sep 17 00:00:00 2001 From: Martin Raspaud Date: Tue, 3 Mar 2026 13:02:13 +0100 Subject: [PATCH 3/4] Add tests --- satpy/dependency_tree.py | 2 +- satpy/tests/test_dependency_tree.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/satpy/dependency_tree.py b/satpy/dependency_tree.py index 8fec4dad66..ebc34db8ed 100644 --- a/satpy/dependency_tree.py +++ b/satpy/dependency_tree.py @@ -550,7 +550,7 @@ def _get_compositor_by_tag(self, key): return preferred # Explicit-tag requests fall back to the first candidate; plain-name requests # return None so that normal name-based lookup can proceed. - return candidates[0] if required_tags else None + return candidates[0] if (required_tags and candidates) else None def _find_tag_candidates(self, standard_name, required_tags): """Return compositors whose standard_name matches and that carry all required_tags.""" diff --git a/satpy/tests/test_dependency_tree.py b/satpy/tests/test_dependency_tree.py index 50d6477cec..879f85e39c 100644 --- a/satpy/tests/test_dependency_tree.py +++ b/satpy/tests/test_dependency_tree.py @@ -139,6 +139,13 @@ class TestGetCompositorByTag: "comp1_crefl", id="preferred_tags_for_plain_name", ), + pytest.param( + {"comp1": []}, + ["wmo"], + "comp1", + "comp1", + id="preferred_tag_unavailable_falls_back_to_plain", + ), ]) def test_get_compositor_by_tag(self, compositors_spec, preferred_tags, query, expected_name): """Test compositor resolution using tag syntax and preferred_composite_tags config.""" @@ -155,6 +162,23 @@ def test_get_compositor_by_tag(self, compositors_spec, preferred_tags, query, ex assert result.attrs["name"] == expected_name + def test_get_compositor_raises_when_explicit_tag_not_found(self): + """Test that requesting a tagged variant that doesn't exist raises KeyError.""" + comp = FakeCompositor(name="comp1", prerequisites=[], standard_name="comp1", tags=[]) + compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1"): comp})} + tree = DependencyTree({}, compositors, {}) + + with pytest.raises(KeyError): + tree.get_compositor(DataQuery(name="comp1:wmo")) + + def test_get_compositor_by_tag_skips_non_string_name(self): + """Test that a non-string name returns None instead of raising TypeError. + + Without the isinstance guard, name.split(":") on None would crash. + """ + tree = DependencyTree({}, {}, {}) + assert tree._get_compositor_by_tag({"name": None}) is None + class TestMissingDependencies(unittest.TestCase): """Test the MissingDependencies exception.""" From 9ebb0d98cb97b82c001ad4435b648808e736bf72 Mon Sep 17 00:00:00 2001 From: Martin Raspaud Date: Wed, 18 Mar 2026 14:45:03 +0100 Subject: [PATCH 4/4] Access composites using tags --- doc/source/composites.rst | 55 ++++++++++++++++--- satpy/dependency_tree.py | 20 +++++++ satpy/enhancements/enhancer.py | 18 +++++- satpy/scene.py | 20 +++++++ .../tests/enhancement_tests/test_enhancer.py | 36 ++++++++++++ satpy/tests/etc/composites/fake_sensor.yaml | 6 ++ satpy/tests/scene_tests/test_load.py | 24 ++++++-- satpy/tests/test_dependency_tree.py | 29 ++++++++++ satpy/tests/test_utils.py | 27 +++++++++ satpy/tests/utils.py | 1 - 10 files changed, 220 insertions(+), 16 deletions(-) diff --git a/doc/source/composites.rst b/doc/source/composites.rst index 5be6ff4f72..133b346eaf 100644 --- a/doc/source/composites.rst +++ b/doc/source/composites.rst @@ -558,13 +558,40 @@ Append ``:`` to the composite name when calling :meth:`~satpy.scene.Scene.l .. code-block:: python - scene.load(["true_color:wmo"]) # loads true_color_wmo + scene.load(["true_color:wmo"]) + + # The dataset is stored and accessible using the same tag syntax: + data = scene["true_color:wmo"] This syntax is interpreted as: "find a compositor with ``standard_name='true_color'`` that has ``'wmo'`` in its ``tags``". It never performs a plain string match, so ``true_color:wmo`` will not accidentally resolve to a compositor named ``true_color`` or ``true_color_wmo``. +Accessing tag-loaded datasets +"""""""""""""""""""""""""""""" + +The dataset is stored in the Scene under the *requested* name (including the +tag suffix) rather than the compositor's internal YAML key. Use the same +tag syntax to retrieve it: + +.. code-block:: python + + scene.load(["true_color:wmo"]) + data = scene["true_color:wmo"] + +The compositor's YAML key name (e.g. ``"true_color_wmo"``) is preserved in +``data.attrs["_satpy_compositor_name"]`` and is used automatically for +enhancement lookups (see below). + +This also means that :attr:`~satpy.scene.Scene.missing_datasets` is empty +after a successful load, and that writing the scene with any writer will use +``"true_color:wmo"`` as the dataset name. + +The same rule applies when a tagged variant is selected automatically via +``preferred_composite_tags``: the dataset is stored under the plain requested +name (e.g. ``"true_color"``), not under the compositor's YAML key name. + Multiple tags can be combined with additional colons. All listed tags must be present on the compositor (AND semantics): @@ -616,12 +643,26 @@ Or as a YAML configuration key: Enhancements for tagged variants ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -No extra configuration is needed on the enhancement side. The enhancement -decision tree already matches by composite ``name`` (the YAML key) *before* -it falls back to ``standard_name``. A composite named ``true_color_wmo`` will -therefore automatically pick up an enhancement entry keyed ``true_color_wmo`` -if one exists, and fall back to the ``standard_name: true_color`` entry -otherwise. +No extra configuration is needed on the enhancement side. When a dataset is +loaded via tag syntax, its ``attrs["_satpy_compositor_name"]`` carries the +compositor's YAML key name (e.g. ``"true_color_wmo"``). The enhancement +lookup tries that name first before falling back to the requested name and +then to ``standard_name``. + +This means an enhancement entry written for ``true_color_wmo`` is picked up +automatically when loading ``"true_color:wmo"``, and enhancements written +against ``standard_name: true_color`` serve as a fallback for all variants. + +.. code-block:: yaml + + # In an enhancement YAML file: + true_color_wmo: # matched via _satpy_compositor_name + name: true_color_wmo + operations: [...] + + true_color_default: # fallback for any true_color variant + standard_name: true_color + operations: [...] Deprecating a composite ------------------------ diff --git a/satpy/dependency_tree.py b/satpy/dependency_tree.py index ebc34db8ed..21bb9e27b2 100644 --- a/satpy/dependency_tree.py +++ b/satpy/dependency_tree.py @@ -446,6 +446,7 @@ def _find_compositor(self, dataset_key, query): root = CompositorNode(compositor) composite_id = root.name + root.name = self._composite_id_with_requested_name(composite_id, dataset_key) prerequisite_filter = composite_id.create_filter_query_without_required_fields(dataset_key) @@ -465,6 +466,25 @@ def _find_compositor(self, dataset_key, query): return root + @staticmethod + def _composite_id_with_requested_name(composite_id, dataset_key): + """Return a DataID matching composite_id but with the name taken from dataset_key. + + This ensures the node in the dependency tree (and eventually the entry in + scene._datasets) uses the name the caller actually requested, including any + tag suffix such as "true_color:high_res", rather than the compositor's + own YAML key name. + """ + try: + requested_name = dataset_key["name"] + except (KeyError, TypeError): + return composite_id + if not isinstance(requested_name, str) or requested_name == composite_id["name"]: + return composite_id + new_id_dict = composite_id.to_dict() + new_id_dict["name"] = requested_name + return composite_id.from_dict(new_id_dict) + def _create_implicit_dependency_subtree(self, dataset_key, query): new_prereq = dataset_key.create_less_modified_query() src_node = self._create_subtree_for_key(new_prereq, query) diff --git a/satpy/enhancements/enhancer.py b/satpy/enhancements/enhancer.py index b196c63e28..3486537030 100644 --- a/satpy/enhancements/enhancer.py +++ b/satpy/enhancements/enhancer.py @@ -84,11 +84,23 @@ def _get_yaml_enhancement_dict(self, config_file: str | Path) -> dict: return enhancement_section def find_match(self, **query_dict): - """Find a match.""" + """Find a match, preferring the YAML compositor key name when available. + + When a dataset was loaded via tag syntax (e.g. ``"true_color:wmo"``), its + ``_satpy_compositor_name`` attribute carries the YAML key name of the compositor + that generated it (e.g. ``"true_color_wmo"``). Enhancement entries are written + against YAML key names, so we try a lookup with that name first before falling + back to the requested name stored in ``name``. + """ + compositor_name = query_dict.get("_satpy_compositor_name") + if compositor_name: + try: + return super().find_match(**{**query_dict, "name": compositor_name}) + except KeyError: + pass try: - return super(EnhancementDecisionTree, self).find_match(**query_dict) + return super().find_match(**query_dict) except KeyError: - # give a more understandable error message raise KeyError("No enhancement configuration found for %s" % (query_dict.get("uid", None),)) diff --git a/satpy/scene.py b/satpy/scene.py index 8693cc77b4..22474b9835 100644 --- a/satpy/scene.py +++ b/satpy/scene.py @@ -1631,6 +1631,7 @@ def _generate_composite(self, comp_node: CompositorNode, keepables: set): optional_datasets=optional_datasets, **comp_node.name.to_dict()) cid = DataID.new_id_from_dataarray(composite) + cid = self._apply_requested_name(comp_node.name, composite, cid) self._datasets[cid] = composite # update the node with the computed DataID @@ -1649,6 +1650,25 @@ def _generate_composite(self, comp_node: CompositorNode, keepables: set): keepables.add(comp_node.name) return + @staticmethod + def _apply_requested_name(requested_id, composite, cid): + """Rename the composite DataArray to the requested name when the compositor overwrote it. + + Compositors write their YAML key name into the output DataArray's ``name`` attr, + overwriting the name that was passed in via ``**comp_node.name.to_dict()``. When this + happens we restore the requested name (which may include a tag suffix such as + ``"true_color:high_res"``) and preserve the YAML key name in + ``attrs["_satpy_compositor_name"]`` so that enhancement lookups can still match entries + written for the YAML key. + """ + requested_name = requested_id.get("name") + yaml_name = cid.get("name") + if requested_name is None or requested_name == yaml_name: + return cid + composite.attrs["_satpy_compositor_name"] = yaml_name + composite.attrs["name"] = requested_name + return DataID.new_id_from_dataarray(composite) + def _get_prereq_datasets(self, comp_id, prereq_nodes, keepables, skip=False): """Get a composite's prerequisites, generating them if needed. diff --git a/satpy/tests/enhancement_tests/test_enhancer.py b/satpy/tests/enhancement_tests/test_enhancer.py index c175f11a9b..8b0d7c0a29 100644 --- a/satpy/tests/enhancement_tests/test_enhancer.py +++ b/satpy/tests/enhancement_tests/test_enhancer.py @@ -479,3 +479,39 @@ def test_reader_and_name_match(self, test_configs_path): img = self._get_enhanced_image(data_arr, test_configs_path) # no reader available, should use default no specified reader np.testing.assert_allclose(img.data.values[0], data_arr.data / 50.0) + + +class TestCompositorNameEnhancementLookup: + """Test that _satpy_compositor_name is used to find enhancement entries keyed by the YAML compositor name.""" + + YAML_KEY_NAME = "true_color_wmo" + REQUESTED_NAME = "true_color:wmo" + + @pytest.fixture + def enh_tree(self): + """Build an EnhancementDecisionTree with an entry keyed by the YAML compositor name only. + + When passing a dict directly, the 'enhancements' wrapper is NOT stripped (unlike YAML files), + so entries must be at the top level. + """ + from satpy.enhancements.enhancer import EnhancementDecisionTree + config = { + self.YAML_KEY_NAME: { + "name": self.YAML_KEY_NAME, + "operations": [{"method": lambda img: None, "args": [], "kwargs": {}}], + }, + } + return EnhancementDecisionTree(config) + + def test_requested_name_alone_does_not_match_yaml_key_entry(self, enh_tree): + """Check that requested tag-syntax name alone cannot match a YAML-key-name enhancement entry.""" + with pytest.raises(KeyError): + enh_tree.find_match(name=self.REQUESTED_NAME) + + def test_find_match_via_compositor_name(self, enh_tree): + """Check that enhancement entry for YAML key is found when _satpy_compositor_name is set on the dataset.""" + result = enh_tree.find_match( + name=self.REQUESTED_NAME, + _satpy_compositor_name=self.YAML_KEY_NAME, + ) + assert result is not None diff --git a/satpy/tests/etc/composites/fake_sensor.yaml b/satpy/tests/etc/composites/fake_sensor.yaml index 82a01db489..2c182a08e9 100644 --- a/satpy/tests/etc/composites/fake_sensor.yaml +++ b/satpy/tests/etc/composites/fake_sensor.yaml @@ -62,6 +62,12 @@ composites: compositor: !!python/name:satpy.tests.utils.FakeCompositor prerequisites: - ds1 + comp1_wmo: + compositor: !!python/name:satpy.tests.utils.FakeCompositor + prerequisites: + - ds1 + standard_name: comp1 + tags: [wmo] comp2: compositor: !!python/name:satpy.tests.utils.FakeCompositor prerequisites: diff --git a/satpy/tests/scene_tests/test_load.py b/satpy/tests/scene_tests/test_load.py index f3e862a46c..9ae5b7f300 100644 --- a/satpy/tests/scene_tests/test_load.py +++ b/satpy/tests/scene_tests/test_load.py @@ -84,7 +84,7 @@ def test_all_datasets_one_reader(self): num_reader_ds = 21 + 6 assert len(id_list) == num_reader_ds id_list = scene.all_dataset_ids(composites=True) - assert len(id_list) == num_reader_ds + 33 + assert len(id_list) == num_reader_ds + 34 def test_all_datasets_multiple_reader(self): """Test all datasets for multiple readers.""" @@ -94,8 +94,8 @@ def test_all_datasets_multiple_reader(self): assert len(id_list) == 2 id_list = scene.all_dataset_ids(composites=True) # ds1 and ds2 => 2 - # composites that use these two datasets => 11 - assert len(id_list) == 2 + 11 + # composites that use these two datasets => 12 + assert len(id_list) == 2 + 12 def test_available_datasets_one_reader(self): """Test the available datasets for one reader.""" @@ -104,8 +104,8 @@ def test_available_datasets_one_reader(self): id_list = scene.available_dataset_ids() assert len(id_list) == 1 id_list = scene.available_dataset_ids(composites=True) - # ds1, comp1, comp14, comp16, static_image, comp26 - assert len(id_list) == 6 + # ds1, comp1, comp1_wmo, comp14, comp16, static_image, comp26 + assert len(id_list) == 7 def test_available_composite_ids_missing_available(self): """Test available_composite_ids when a composites dep is missing.""" @@ -317,6 +317,20 @@ def test_single_composite_loading(self, comp_name, exp_id_or_name): else: assert loaded_ids[0] == exp_id_or_name + def test_load_composite_by_tag_syntax(self): + """Check that loading with tag syntax stores the dataset under the requested name and makes it accessible. + + The compositor's self.attrs['name'] (the YAML key, e.g. 'comp1_wmo') overwrites the + requested name in the output attrs — exactly as GenericCompositor does. + _generate_composite must restore the requested name ('comp1:wmo') and preserve the + original YAML key in _satpy_compositor_name for enhancement lookups. + """ + scene = Scene(filenames=["fake1_1.txt"], reader="fake1") + scene.load(["comp1:wmo"]) + assert not scene.missing_datasets + data = scene["comp1:wmo"] + assert data.attrs["_satpy_compositor_name"] == "comp1_wmo" + def test_load_multiple_resolutions(self): """Test loading a dataset has multiple resolutions available with different resolutions.""" scene = Scene(filenames=["fake1_1.txt"], reader="fake1") diff --git a/satpy/tests/test_dependency_tree.py b/satpy/tests/test_dependency_tree.py index 879f85e39c..20d05b8676 100644 --- a/satpy/tests/test_dependency_tree.py +++ b/satpy/tests/test_dependency_tree.py @@ -180,6 +180,35 @@ def test_get_compositor_by_tag_skips_non_string_name(self): assert tree._get_compositor_by_tag({"name": None}) is None +class TestFindCompositorNodeNaming: + """Test that compositor nodes are named after the requested dataset key.""" + + def test_tag_request_names_node_after_request(self): + """Check that a tag-syntax request resolves to a node named after the requested key, not the YAML key.""" + comp = FakeCompositor(name="comp1_wmo", prerequisites=[], standard_name="comp1", tags=["wmo"]) + compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo"): comp})} + tree = DependencyTree({}, compositors, {}) + + requested = {DataQuery(name="comp1:wmo")} + tree.populate_with_keys(requested) + + resolved_name = next(iter(requested)) + assert resolved_name["name"] == "comp1:wmo" + + def test_preferred_tag_resolution_names_node_after_plain_request(self): + """Check that a plain request resolved via preferred_composite_tags is named after the plain request.""" + comp_wmo = FakeCompositor(name="comp1_wmo", prerequisites=[], standard_name="comp1", tags=["wmo"]) + compositors = {"fake_sensor": DatasetDict({make_cid(name="comp1_wmo"): comp_wmo})} + tree = DependencyTree({}, compositors, {}) + + requested = {DataQuery(name="comp1")} + with satpy.config.set(preferred_composite_tags=["wmo"]): + tree.populate_with_keys(requested) + + resolved_name = next(iter(requested)) + assert resolved_name["name"] == "comp1" + + class TestMissingDependencies(unittest.TestCase): """Test the MissingDependencies exception.""" diff --git a/satpy/tests/test_utils.py b/satpy/tests/test_utils.py index fbb917b948..41213351cc 100644 --- a/satpy/tests/test_utils.py +++ b/satpy/tests/test_utils.py @@ -662,3 +662,30 @@ def test_flatten_dict(): "b_d_e": 1, "b_d_f_g": [1, 2]} assert flatten_dict(d) == expected + + +class TestFakeCompositor: + """Test that FakeCompositor metadata priority is consistent with real compositors.""" + + def test_yaml_name_wins_over_kwargs_name(self): + """FakeCompositor output attrs['name'] must equal self.attrs['name'], not the kwargs name. + + Real compositors (GenericCompositor) do new_attrs.update(self.attrs) last, so + self.attrs["name"] (the YAML key) always overwrites any name passed as a kwarg. + FakeCompositor must follow the same rule. + """ + from unittest.mock import MagicMock + + from satpy.tests.utils import FakeCompositor + + area = MagicMock() + comp = FakeCompositor(name="comp1_wmo", prerequisites=["prereq"], standard_name="comp1") + prereq = xr.DataArray( + da.zeros((4, 5)), + dims=["y", "x"], + attrs={"area": area}, + ) + result = comp([prereq], name="comp1:wmo") + assert result.attrs["name"] == "comp1_wmo", ( + "FakeCompositor must use self.attrs['name'] (the YAML key), not the kwargs name" + ) diff --git a/satpy/tests/utils.py b/satpy/tests/utils.py index e37ced2b6e..8ab97b5639 100644 --- a/satpy/tests/utils.py +++ b/satpy/tests/utils.py @@ -199,7 +199,6 @@ def __call__(self, projectables, nonprojectables=None, **kwargs): if len(projectables) != len(self.attrs["prerequisites"]): raise ValueError("Not enough prerequisite datasets passed") - info.update(kwargs) if projectables: info["area"] = projectables[0].attrs["area"] dim_sizes = projectables[0].sizes