used @maxnoe's suggestion of a Deprecated wrapper class

- added Deprecated
- changed deprecated members to use it
- also added  a "version" attribute
- added test

ctapipe/core/__init__.py

@@ -4,7 +4,7 @@
 """
 
 from .component import Component, non_abstract_children
-from .container import Container, Field, DeprecatedField, Map
+from .container import Container, Field, Deprecated, Map
 from .provenance import Provenance, get_module_version
 from .tool import Tool, ToolConfigurationError
 
@@ -13,7 +13,7 @@
     'Container',
     'Tool',
     'Field',
-    'DeprecatedField',
+    'Deprecated',
     'Map',
     'Provenance',
     'ToolConfigurationError',

ctapipe/core/container.py

@@ -2,6 +2,54 @@
 from copy import deepcopy
 from pprint import pformat
 from textwrap import wrap
+import warnings
+
+
+class Deprecated:
+    """
+    Decorate a field that is to be deprecated. Issues a UserWarning if the Field is
+    accessed.
+
+    Parameters
+    ----------
+    field: Field
+        Field class to deprecate
+    help: str
+        Description of what the user should use instead, or reason for deprecation
+    version:
+        version number above which this is deprecated.
+
+    Examples
+    --------
+    .. code-block: python
+        my_field = Deprecated(Field(default=None), help='Use B.field instead.')
+    """
+
+    def __init__(self, field, help="", version="next"):
+        self.field = field
+        self.help = help
+        self.value = field.default
+        self.version = version
+
+    def __set_name__(self, owner, name):
+        self.owner = owner
+        self.name = name
+
+    def __get__(self, instance, owner=None):
+        warnings.warn(
+            f"Using field {self.name} of container {self.owner}"
+            f" is deprecated (since v{self.version}. {self.help}",
+            DeprecationWarning,
+        )
+        return self.value
+
+    def __set__(self, instance, value):
+        warnings.warn(
+            f"Using field {self.name} of container {self.owner}"
+            f" is deprecated. {self.help}",
+            DeprecationWarning,
+        )
+        self.value = value
 
 
 class Field:
@@ -34,12 +82,6 @@ def __repr__(self):
         return desc
 
 
-class DeprecatedField(Field):
-    """ used to mark which fields may be removed in next version """
-
-    pass
-
-
 class ContainerMeta(type):
     """
     The MetaClass for the Containers

ctapipe/core/tests/test_container.py

@@ -1,52 +1,51 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 import pytest
-from ctapipe.core import Container, Field, Map
+from ctapipe.core import Container, Field, Map, Deprecated
 
 
 def test_prefix():
     class AwesomeContainer(Container):
         pass
 
     # make sure the default prefix is class name without container
-    assert AwesomeContainer.container_prefix == 'awesome'
-    assert AwesomeContainer().prefix == 'awesome'
+    assert AwesomeContainer.container_prefix == "awesome"
+    assert AwesomeContainer().prefix == "awesome"
 
     # make sure we can set the class level prefix at definition time
     class ReallyAwesomeContainer(Container):
-        container_prefix = 'test'
+        container_prefix = "test"
 
-    assert ReallyAwesomeContainer.container_prefix == 'test'
+    assert ReallyAwesomeContainer.container_prefix == "test"
     r = ReallyAwesomeContainer()
-    assert r.prefix == 'test'
+    assert r.prefix == "test"
 
-    ReallyAwesomeContainer.container_prefix = 'test2'
+    ReallyAwesomeContainer.container_prefix = "test2"
     # new instance should have the old prefix, old instance
     # the one it was created with
-    assert ReallyAwesomeContainer().prefix == 'test2'
-    assert r.prefix == 'test'
+    assert ReallyAwesomeContainer().prefix == "test2"
+    assert r.prefix == "test"
 
     # Make sure we can set the class level prefix at runtime
-    ReallyAwesomeContainer.container_prefix = 'foo'
-    assert ReallyAwesomeContainer().prefix == 'foo'
+    ReallyAwesomeContainer.container_prefix = "foo"
+    assert ReallyAwesomeContainer().prefix == "foo"
 
     # make sure we can assign instance level prefixes
     c1 = ReallyAwesomeContainer()
     c2 = ReallyAwesomeContainer()
-    c2.prefix = 'c2'
+    c2.prefix = "c2"
 
-    assert c1.prefix == 'foo'
-    assert c2.prefix == 'c2'
+    assert c1.prefix == "foo"
+    assert c2.prefix == "c2"
 
 
 def test_inheritance():
-
     class ExampleContainer(Container):
         a = Field(None)
 
     class SubclassContainer(ExampleContainer):
         b = Field(None)
 
-    assert 'a' in SubclassContainer.fields
+    assert "a" in SubclassContainer.fields
 
     c = SubclassContainer()
     assert c.a is None
@@ -60,7 +59,6 @@ class SubclassContainer(ExampleContainer):
 
 
 def test_multiple_inheritance():
-
     class ContainerA(Container):
         a = Field(None)
 
@@ -70,12 +68,11 @@ class ContainerB(ContainerA):
     class ContainerC(ContainerB):
         c = Field(None)
 
-    assert 'a' in ContainerC.fields
-    assert 'b' in ContainerC.fields
+    assert "a" in ContainerC.fields
+    assert "b" in ContainerC.fields
 
 
 def test_override_inheritance():
-
     class ContainerA(Container):
         a = Field(1)
 
@@ -90,7 +87,6 @@ class ContainerB(ContainerA):
 
 
 def test_container():
-
     class ExampleContainer(Container):
         x = Field(-1, "x value")
         y = Field(-1, "y value")
@@ -117,8 +113,8 @@ class ExampleContainer(Container):
     assert cont.x == -1
 
     # test adding metadata
-    cont.meta['stuff'] = 'things'
-    assert 'stuff' in cont.meta and cont.meta['stuff'] == 'things'
+    cont.meta["stuff"] = "things"
+    assert "stuff" in cont.meta and cont.meta["stuff"] == "things"
 
 
 def test_child_containers():
@@ -134,7 +130,6 @@ class ParentContainer(Container):
 
 
 def test_map_containers():
-
     class ChildContainer(Container):
         z = Field(1, "sub-item")
 
@@ -154,7 +149,6 @@ class ParentContainer(Container):
 
 
 def test_container_as_dict():
-
     class ChildContainer(Container):
         z = Field(1, "sub-item")
 
@@ -167,18 +161,28 @@ class ParentContainer(Container):
     the_flat_dict = cont.as_dict(recursive=True, flatten=True)
     the_dict = cont.as_dict(recursive=True, flatten=False)
 
-    assert 'child_z' in the_flat_dict
-    assert 'child' in the_dict and 'z' in the_dict['child']
+    assert "child_z" in the_flat_dict
+    assert "child" in the_dict and "z" in the_dict["child"]
 
 
 def test_container_brackets():
-
-    class TestContainer(Container):
+    class ExampleContainer(Container):
         answer = Field(-1, "The answer to all questions")
 
-    t = TestContainer()
+    t = ExampleContainer()
 
-    t['answer'] = 42
+    t["answer"] = 42
 
     with pytest.raises(AttributeError):
-        t['foo'] = 5
+        t["foo"] = 5
+
+
+def test_deprecated_fields():
+    class ExampleContainer(Container):
+        deprecated_field = Deprecated(
+            Field(-1, "going away"), help="replaced later", version="1.0.0"
+        )
+
+    cont = ExampleContainer()
+    with pytest.warns(DeprecationWarning):
+        cont.deprecated_field = 6

ctapipe/io/containers.py

@@ -7,7 +7,7 @@
 from astropy.time import Time
 from numpy import nan
 
-from ..core import Container, Field, DeprecatedField, Map
+from ..core import Container, Field, Deprecated, Map
 from ..instrument import SubarrayDescription
 
 __all__ = [
@@ -162,8 +162,8 @@ class R0Container(Container):
     Storage of a Merged Raw Data Event
     """
 
-    obs_id = DeprecatedField(-1, "observation ID")
-    event_id = DeprecatedField(-1, "event id number")
+    obs_id = Deprecated(Field(-1, "observation ID"), "use event.index instead")
+    event_id = Deprecated(Field(-1, "event id number"), "use event.index instead")
     tels_with_data = Field([], "list of telescopes with data")
     tel = Field(Map(R0CameraContainer), "map of tel_id to R0CameraContainer")
 
@@ -197,8 +197,8 @@ class R1Container(Container):
     Storage of a r1 calibrated Data Event
     """
 
-    obs_id = DeprecatedField(-1, "observation ID")
-    event_id = DeprecatedField(-1, "event id number")
+    obs_id = Deprecated(Field(-1, "observation ID"), "use event.index instead")
+    event_id = Deprecated(Field(-1, "event id number", "use event.index instead"))
     tels_with_data = Field([], "list of telescopes with data")
     tel = Field(Map(R1CameraContainer), "map of tel_id to R1CameraContainer")
 
@@ -227,8 +227,8 @@ class DL0Container(Container):
     Storage of a data volume reduced Event
     """
 
-    obs_id = DeprecatedField(-1, "observation ID")  # use event.index.obs_id
-    event_id = DeprecatedField(-1, "event id number")  # use event.index.event_id
+    obs_id = Deprecated(Field(-1, "observation ID"), "use event.index instead")
+    event_id = Deprecated(Field(-1, "event id number"), "use event.index instead")
     tels_with_data = Field([], "list of telescopes with data")
     tel = Field(Map(DL0CameraContainer), "map of tel_id to DL0CameraContainer")
 
@@ -482,8 +482,10 @@ class DataContainer(Container):
     mcheader = Field(MCHeaderContainer(), "Monte-Carlo run header data")
     trig = Field(CentralTriggerContainer(), "central trigger information")
     count = Field(0, "number of events processed")
-    inst = DeprecatedField(
-        InstrumentContainer(), "instrumental information (deprecated"
+    inst = Deprecated(
+        Field(InstrumentContainer(), "instrumental information"),
+        help="to be moved outside of event structure",
+        version="1.0.0",
     )
     pointing = Field(Map(TelescopePointingContainer), "Telescope pointing positions")
 
@@ -577,9 +579,15 @@ class MuonIntensityParameter(Container):
 
     """
 
-    obs_id = DeprecatedField(0, "run identification number")
-    event_id = DeprecatedField(0, "event identification number")
-    tel_id = DeprecatedField(0, "telescope identification number")
+    obs_id = Deprecated(
+        Field(0, "run identification number"), "use event.index instead"
+    )
+    event_id = Deprecated(
+        Field(0, "event identification number"), "use event.index instead"
+    )
+    tel_id = Deprecated(
+        Field(0, "telescope identification number"), "use event.index instead"
+    )
     ring_completeness = Field(0.0, "fraction of ring present")
     ring_pix_completeness = Field(0.0, "fraction of pixels present in the ring")
     ring_num_pixel = Field(0, "number of pixels in the ring image")
@@ -819,7 +827,7 @@ class WaveformCalibrationContainer(Container):
         "np array of (digital count) to (photon electron) coefficients (n_chan, n_pix)",
     )
 
-    pedestal_per_sample =    Field(
+    pedestal_per_sample = Field(
         None,
         "np array of average pedestal value per sample (digital count) (n_chan, n_pix)",
     )