From 2d6db93f5888e3100d9264503b437984da593c12 Mon Sep 17 00:00:00 2001
From: richard gowers <richardjgowers@gmail.com>
Date: Wed, 13 Mar 2024 11:25:43 +0000
Subject: [PATCH] add a deprecation warning for old-style mapping as dict input

pr #260 changed the signature of Transformation.__init__ and Protocol.create to take mapping as list (or singular) rather than dict.

This adds an implicit conversion with deprecation warning

this should smooth over old scripts which still use a dict
---
 gufe/protocols/protocol.py             |  9 ++++++++-
 gufe/tests/test_protocol.py            |  9 +++++++++
 gufe/tests/test_transformation.py      | 14 ++++++++++++++
 gufe/transformations/transformation.py |  9 ++++++++-
 4 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/gufe/protocols/protocol.py b/gufe/protocols/protocol.py
index a36b62ae..bfda10c8 100644
--- a/gufe/protocols/protocol.py
+++ b/gufe/protocols/protocol.py
@@ -8,6 +8,7 @@
 import abc
 from typing import Optional, Iterable, Any, Union
 from openff.units import Quantity
+import warnings
 
 from ..settings import Settings, SettingsBaseModel
 from ..tokenization import GufeTokenizable, GufeKey
@@ -176,7 +177,7 @@ def create(
         *,
         stateA: ChemicalSystem,
         stateB: ChemicalSystem,
-        mapping: Optional[Union[ComponentMapping, list[ComponentMapping]]],
+        mapping: Optional[Union[ComponentMapping, list[ComponentMapping], dict[str, ComponentMapping]]],
         extends: Optional[ProtocolDAGResult] = None,
         name: Optional[str] = None,
         transformation_key: Optional[GufeKey] = None
@@ -219,6 +220,12 @@ def create(
         ProtocolDAG
             A directed, acyclic graph that can be executed by a `Scheduler`.
         """
+        if isinstance(mapping, dict):
+            warnings.warn(("mapping input as a dict is deprecated, "
+                           "instead use either a single Mapping or list"),
+                          DeprecationWarning)
+            mapping = list(mapping.values())
+
         return ProtocolDAG(
             name=name,
             protocol_units=self._create(
diff --git a/gufe/tests/test_protocol.py b/gufe/tests/test_protocol.py
index 32d3ff88..5ce1aa49 100644
--- a/gufe/tests/test_protocol.py
+++ b/gufe/tests/test_protocol.py
@@ -333,6 +333,15 @@ def test_create_execute_gather(self, protocol_dag):
 
         assert protocolresult.get_estimate() == 95500.0
 
+    def test_deprecation_warning_on_dict_mapping(self, instance, vacuum_ligand, solvated_ligand):
+        lig = solvated_ligand.components['ligand']
+        mapping = gufe.LigandAtomMapping(lig, lig, componentA_to_componentB={})
+
+        with pytest.warns(DeprecationWarning,
+                          match="mapping input as a dict is deprecated"):
+            instance.create(stateA=solvated_ligand, stateB=vacuum_ligand,
+                            mapping={'ligand': mapping})
+
     class ProtocolDAGTestsMixin(GufeTokenizableTestsMixin):
         
         def test_protocol_units(self, instance):
diff --git a/gufe/tests/test_transformation.py b/gufe/tests/test_transformation.py
index 38182b4c..ef10a562 100644
--- a/gufe/tests/test_transformation.py
+++ b/gufe/tests/test_transformation.py
@@ -5,6 +5,7 @@
 import io
 import pathlib
 
+import gufe
 from gufe.transformations import Transformation, NonTransformation
 from gufe.protocols.protocoldag import execute_DAG
 
@@ -123,6 +124,19 @@ def test_dump_load_roundtrip(self, absolute_transformation):
         recreated = Transformation.load(string)
         assert absolute_transformation == recreated
 
+    def test_deprecation_warning_on_dict_mapping(self, solvated_ligand, solvated_complex):
+        lig = solvated_complex.components['ligand']
+        # this mapping makes no sense, but it'll trigger the dep warning we want
+        mapping = gufe.LigandAtomMapping(lig, lig, componentA_to_componentB={})
+
+        with pytest.warns(DeprecationWarning,
+                          match="mapping input as a dict is deprecated"):
+            Transformation(
+                solvated_complex, solvated_ligand,
+                protocol=DummyProtocol(settings=DummyProtocol.default_settings()),
+                mapping={'ligand': mapping},
+            )
+
 
 class TestNonTransformation(GufeTokenizableTestsMixin):
 
diff --git a/gufe/transformations/transformation.py b/gufe/transformations/transformation.py
index af0ac91a..58a898be 100644
--- a/gufe/transformations/transformation.py
+++ b/gufe/transformations/transformation.py
@@ -3,6 +3,7 @@
 
 from typing import Optional, Iterable, Union
 import json
+import warnings
 
 from ..tokenization import GufeTokenizable, JSON_HANDLER
 from ..utils import ensure_filelike
@@ -24,7 +25,7 @@ def __init__(
         stateA: ChemicalSystem,
         stateB: ChemicalSystem,
         protocol: Protocol,
-        mapping: Optional[Union[ComponentMapping, list[ComponentMapping]]] = None,
+        mapping: Optional[Union[ComponentMapping, list[ComponentMapping], dict[str, ComponentMapping]]] = None,
         name: Optional[str] = None,
     ):
         """Two chemical states with a method for estimating free energy difference
@@ -47,6 +48,12 @@ def __init__(
         name : str, optional
            a human-readable tag for this transformation
         """
+        if isinstance(mapping, dict):
+            warnings.warn(("mapping input as a dict is deprecated, "
+                           "instead use either a single Mapping or list"),
+                          DeprecationWarning)
+            mapping = list(mapping.values())
+
         self._stateA = stateA
         self._stateB = stateB
         self._mapping = mapping