Repository: microsoft/pybryt

Issue:
Simplified user interface for temporal annotations
**Is your feature request related to a problem? Please describe.**
We find the current interface for defining temporal annotations a bit too complicated and error-prone. Have you considered the solution to define a "container" to which individual value annotations are added? The order of annotations in the container would define "before" and "after" relationships. This could also simplify the way of defining `success_message` and `failure_message` for temporal annotations.

An example of this code can be found as an example `04-temporal-annotations` in https://github.com/marijanbeg/pybryt-examples repository.

**Describe the solution you'd like**
The current reference implementation for the Lucas series using temporal annotations is:
```python
import pybryt
import numpy as np


def lucas(n):
    lucas_series = np.zeros(n, dtype=int)

    lucas_series[0] = 2
    curr_value = pybryt.Value(lucas_series,
                              name='first_element',
                              success_message='SUCCESS 1: Your first element is correct.',
                              failure_message='ERROR 1: Please check your first element in the series.')
    if n == 1:
        return lucas_series

    lucas_series[1] = 1
    new_value = pybryt.Value(lucas_series,
                             name='second_element',
                             success_message='SUCCESS 2: Your second element is correct.',
                             failure_message='ERROR 2: Please check your second element in the series.')
    curr_value.before(new_value)
    curr_value = new_value
    if n == 2:
        return lucas_series

    for i in range(2, n):
        lucas_series[i] = lucas_series[i-1] + lucas_series[i-2]
        new_value = pybryt.Value(lucas_series,
                                 name='other_elements',
                                 success_message='SUCCESS 3: You are generating n>2 elements right.',
                                 failure_message='ERROR 3: Hmmm... Are you summing the previous two elements right?')
        curr_value.before(new_value)
        curr_value = new_value

    return lucas_series


pybryt.Value(lucas(20),
             name='final',
             success_message='SUCCESS 4: Amazing! Your final solution is correct.',
             failure_message='ERROR 4: The Lucas series you computed is wrong.')
```

A suggestion for a possible simplification could be:

```python
# This code is not working. It is a suggestion for defining temporal annotations.
import pybryt
import numpy as np


def lucas(n):
    container = pybryt.Container(sucess_message='SUCCESS 0: ...',
                                 failure_message='ERROR 0: ...')  # Container
    lucas_series = np.zeros(n, dtype=int)

    lucas_series[0] = 2
    container.add(pybryt.Value(lucas_series,
                               name='first_element',
                               success_message='SUCCESS 1: Your first element is correct.',
                               failure_message='ERROR 1: Please check your first element in the series.'))
    if n == 1:
        return lucas_series

    lucas_series[1] = 1
    container.add(pybryt.Value(lucas_series,
                              name='second_element',
                              success_message='SUCCESS 2: Your second element is correct.',
                              failure_message='ERROR 2: Please check your second element in the series.'))
    
    if n == 2:
        return lucas_series

    for i in range(2, n):
        lucas_series[i] = lucas_series[i-1] + lucas_series[i-2]
        container.add(pybryt.Value(lucas_series,
                                  name='other_elements',
                                  success_message='SUCCESS 3: You are generating n>2 elements right.',
                                  failure_message='ERROR 3: Hmmm... Are you summing the previous two elements right?'))

    return lucas_series


pybryt.Value(lucas(20),
             name='final',
             success_message='SUCCESS 4: Amazing! Your final solution is correct.',
             failure_message='ERROR 4: The Lucas series you computed is wrong.')
```

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 74b5136..6576baf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,10 @@
 All notable changes to this project will be documented in this file, and this project adheres to 
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+* Added the `Collection` annotation
+
 ## 0.1.2 - 2021-06-14
 
 * Added the `ForbidType` annotation
diff --git a/docs/annotations/collections.rst b/docs/annotations/collections.rst
new file mode 100644
index 0000000..999fa36
--- /dev/null
+++ b/docs/annotations/collections.rst
@@ -0,0 +1,69 @@
+.. _collections:
+
+Annotation Collections
+======================
+
+To faciliate the easy grouping and management of annotations, PyBryt provides the 
+:py:class:`Collection<pybryt.annotations.collection.Collection>` annotation, which collects a series
+of other annotations into a group. Collections can be instantiated by passing zero or more
+annotations to the constructor, along with the other usual keyword arguments for other annotations.
+To add additional annotations to the collection, use 
+:py:meth:`Collection.add<pybryt.annotations.collection.Collection.add>`.
+
+.. code-block:: python
+
+    l = [3, 2, 1, 4, -3, -3, 3, 4]
+
+    collection = pybryt.Collection(pybryt.Value(l))
+
+    # calculate the partial products of a list
+    prod = 1
+    for i in range(len(l)):
+        prod *= l[i]
+        l[i] = prod
+        collection.add(pybryt.value(l))
+
+Annotations can be removed from the collection using
+:py:meth:`Collection.remove<pybryt.annotations.collection.Collection.remove>`:
+
+.. code-block:: python
+
+    val = pybryt.Value(l)
+    collection = pybryt.Collection(val)
+    collection.remove(val)
+
+Collection annotations can be used to simplify the creation of temporal annotations. Rather than
+reassigning variables and using the ``before`` method over and over again, you can simply create
+a collection and set ``enforce_order=True`` in the constructor. This will mean that the collection
+is only satified if the satisfying timestamps of its annotations occur in non-decreasing order based
+on the order the annotations were added in. For the example above, this can be done with
+
+.. code-block:: python
+
+    l = [3, 2, 1, 4, -3, -3, 3, 4]
+
+    collection = pybryt.Collection(pybryt.Value(l), enforce_order=True)  # this is the only difference!
+
+    # calculate the partial products of a list
+    prod = 1
+    for i in range(len(l)):
+        prod *= l[i]
+        l[i] = prod
+        collection.add(pybryt.value(l))
+
+Collections can also be used to simplify the management of success and failure messages by linking
+them to a specific group of annotations:
+
+.. code-block:: python
+
+    l = [3, 2, 1, 4, -3, -3, 3, 4]
+
+    collection = pybryt.Collection(pybryt.Value(l), enforce_order=True,
+                                   failure_message="Take a look at your partial product algorithm.")
+
+    # calculate the partial products of a list
+    prod = 1
+    for i in range(len(l)):
+        prod *= l[i]
+        l[i] = prod
+        collection.add(pybryt.value(l))
diff --git a/docs/annotations/index.rst b/docs/annotations/index.rst
index 6202194..f56b0a8 100644
--- a/docs/annotations/index.rst
+++ b/docs/annotations/index.rst
@@ -11,6 +11,7 @@ Annotations
     relational_annotations
     complexity_annotations
     type_annotations
+    collections
     invariants
 
 Annotations are the basic building blocks, out of which reference
@@ -24,12 +25,13 @@ boolean logic surrounding the presence or absence of those values.
 
 All annotations are created by instantiating subclasses of the abstract
 :py:class:`Annotation<pybryt.annotations.annotation.Annotation>` class. There 
-are four main types of annotations: 
+are five main types of annotations: 
  
 * :ref:`value annotations<value>`
 * :ref:`relational annotations<relational>`
 * :ref:`complexity annotations<complexity>`
 * :ref:`type annotations<type>`
+* :ref:`annotation collections<collection>`
 
 
 Annotation Arguments
diff --git a/docs/annotations/relational_annotations.rst b/docs/annotations/relational_annotations.rst
index 17e2254..350469f 100644
--- a/docs/annotations/relational_annotations.rst
+++ b/docs/annotations/relational_annotations.rst
@@ -14,6 +14,7 @@ All relational annotations are subclasses of the abstract
 class, which defines some helpful defaults for working with annotations that
 have child annotations.
 
+
 Temporal Annotations
 --------------------
 
@@ -81,6 +82,7 @@ instance of the
 :py:class:`BeforeAnnotation<pybryt.annotations.relation.BeforeAnnotation>`
 class, but with the operands switched.
 
+
 Boolean Annotations
 -------------------
 
diff --git a/docs/api_reference.rst b/docs/api_reference.rst
index c3fc3d8..e8aedec 100644
--- a/docs/api_reference.rst
+++ b/docs/api_reference.rst
@@ -62,6 +62,14 @@ Type Annotations
    :undoc-members:
 
 
+Annotation Collections
+++++++++++++++++++++++
+
+.. automodule:: pybryt.annotations.collection
+   :members:
+   :undoc-members:
+
+
 Annotation Results
 ++++++++++++++++++
 
diff --git a/pybryt/annotations/__init__.py b/pybryt/annotations/__init__.py
index 093e6b0..1db514b 100644
--- a/pybryt/annotations/__init__.py
+++ b/pybryt/annotations/__init__.py
@@ -1,7 +1,8 @@
 """Annotations for marking-up reference implementations"""
 
 from . import invariants
-from .annotation import Annotation, AnnotationResult
+from .annotation import *
+from .collection import *
 from .complexity import *
 from .relation import *
 from .type_ import *
diff --git a/pybryt/annotations/annotation.py b/pybryt/annotations/annotation.py
index d6e9591..9cd5c1d 100644
--- a/pybryt/annotations/annotation.py
+++ b/pybryt/annotations/annotation.py
@@ -1,5 +1,7 @@
 """Abstract base class for annotations and annotation results class"""
 
+__all__ = ["Annotation", "AnnotationResult"]
+
 from abc import ABC, abstractmethod
 from collections import OrderedDict
 from typing import Any, Dict, List, NoReturn, Optional, Tuple
diff --git a/pybryt/annotations/collection.py b/pybryt/annotations/collection.py
new file mode 100644
index 0000000..9428593
--- /dev/null
+++ b/pybryt/annotations/collection.py
@@ -0,0 +1,138 @@
+"""Class for gathering and operating on collections of annotations"""
+
+__all__ = ["Collection"]
+
+from typing import Any, Dict, List, NoReturn, Tuple
+
+from .annotation import Annotation, AnnotationResult
+
+
+class Collection(Annotation):
+    """
+    A class for collecting and operating on multiple annotations.
+
+    If ``enforce_order`` is true, this collection will only be satisfied if all of its children are
+    satisfied *and* the satisfying timestamps are in non-decreasing order (for those that have them).
+
+    Args:
+        *annotations (:py:class:`Annotation<pybryt.annotations.annotation.Annotation>`): the child 
+            annotations being operated on
+        enforce_order (``bool``, optional): whether to enforce the ordering of annotations as added 
+            to this collection
+        **kwargs: additional keyword arguments passed to the 
+            :py:class:`Annotation<pybryt.annotations.annotation.Annotation>` constructor
+    """
+
+    _annotations: List[Annotation]
+    """the child annotations that this annotation operates on"""
+
+    enforce_order: bool
+    """whether to enforce the ordering of annotations as added to this collection"""
+
+    def __init__(self, *annotations: Annotation, enforce_order: bool = False, **kwargs):
+        self._annotations = list(annotations)
+        for ann in self._annotations:
+            if not isinstance(ann, Annotation):
+                raise ValueError("One of the arguments is not an annotation")
+
+        self.enforce_order = enforce_order
+
+        super().__init__(**kwargs)
+
+    @property
+    def children(self) -> List[Annotation]:
+        """
+        ``list[Annotation]``: the child annotations that this annotation operates on
+        """
+        return self._annotations
+
+    def __eq__(self, other: Any) -> bool:
+        """
+        Checks whether this annotation is equal to another object.
+
+        For an object to equal a relational annotation, it must also be a relational annotation of
+        the same type and have the same child annotations.
+
+        Args:
+            other (``object``): the object to compare to
+
+        Returns:
+            ``bool``: whether the objects are equal
+        """
+        return super().__eq__(other) and self.children == other.children and \
+            self.enforce_order == other.enforce_order
+
+    def check(self, observed_values: List[Tuple[Any, int]]) -> AnnotationResult:
+        """
+        Checks that all child annotations are satisfied by the values in ``observed_values``, and
+        that the timestamps of the satisfying values occur in non-decreasing order if 
+        ``self.enforce_order`` is true.
+
+        Args:
+            observed_values (``list[tuple[object, int]]``): a list of tuples of values observed
+                during execution and the timestamps of those values
+        
+        Returns:
+            :py:class:`AnnotationResult`: the results of this annotation based on 
+            ``observed_values``
+        """
+        results = []
+        for ann in self.children:
+            results.append(ann.check(observed_values))
+        
+        if self.enforce_order and all(res.satisfied for res in results):
+                before = []
+                with_timestamp = [res for res in results if res.satisfied_at != -1]
+                for i in range(len(with_timestamp) - 1):
+                    before.append(with_timestamp[i].satisfied_at < with_timestamp[i + 1].satisfied_at)
+
+                return AnnotationResult(all(before), self, children = results)
+
+        else:
+            return AnnotationResult(None, self, children = results)
+
+    def to_dict(self) -> Dict[str, Any]:
+        """
+        Converts this annotation's details to a JSON-friendly dictionary format.
+
+        Returns:
+            ``dict[str, object]``: the dictionary representation of this annotation
+        """
+        d = super().to_dict()
+        d.update({
+            "enforce_order": self.enforce_order,
+        })
+        return d
+
+    def add(self, annotation: Annotation) -> NoReturn:
+        """
+        Adds an annotation to this collection.
+
+        Args:
+            annotation (:py:class:`Annotation<pybryt.annotations.annotation.Annotation>`): the
+                annotation to add
+        """
+        if not isinstance(annotation, Annotation):
+            raise TypeError(f"{annotation} is not an annotation")
+
+        self._annotations.append(annotation)
+        try:
+            self.get_tracked_annotations().remove(annotation)
+        except ValueError:  # pragma: no cover
+            pass
+
+    def remove(self, annotation: Annotation) -> NoReturn:
+        """
+        Removes an annotation from this collection.
+
+        Args:
+            annotation (:py:class:`Annotation<pybryt.annotations.annotation.Annotation>`): the
+                annotation to remove
+        """
+        if not isinstance(annotation, Annotation):
+            raise TypeError(f"{annotation} is not an annotation")
+
+        if annotation not in self._annotations:
+            raise ValueError(f"The specified annotation is not part of this collection")
+
+        self._annotations.remove(annotation)


Test patch:
diff --git a/tests/annotations/test_collection.py b/tests/annotations/test_collection.py
new file mode 100644
index 0000000..dacd020
--- /dev/null
+++ b/tests/annotations/test_collection.py
@@ -0,0 +1,67 @@
+"""Tests for annotation collections"""
+
+import time
+import pytest
+
+from unittest import mock
+
+from pybryt import *
+from pybryt.utils import pickle_and_hash
+
+from .utils import *
+
+
+def test_collection():
+    """
+    """
+    mfp = generate_memory_footprint()
+    Annotation.reset_tracked_annotations()
+
+    v1, v2 = Value(mfp[0][0]), Value(mfp[2][0])
+    a = Collection(v1, v2, success_message="foo")
+
+    res = a.check(mfp)
+    check_obj_attributes(a, {"children__len": 2})
+    check_obj_attributes(res, {
+        "children__len": 2,
+        "satisfied": True,
+        "_satisfied": None,
+        "annotation": a,
+        "timestamp": -1,
+        "satisfied_at": 2,
+    })
+
+    assert a.to_dict() == {
+        "name": "Annotation 3",
+        "children": [v1.to_dict(), v2.to_dict()],
+        "success_message": "foo",
+        "failure_message": None,
+        "limit": None,
+        "group": None,
+        "type": "collection",
+        "enforce_order": False,
+    }
+
+    a = Collection(v1, v2, enforce_order=True, success_message="foo")
+    res = a.check(mfp)
+    assert res.satisfied
+
+    v3 = Value(mfp[1][0])
+    a.add(v3)
+    res = a.check(mfp)
+    assert not res.satisfied
+
+    a.remove(v3)
+    res = a.check(mfp)
+    assert res.satisfied
+
+    a = Collection(v2, v1, enforce_order=True, success_message="foo")
+    res = a.check(mfp)
+    assert not res.satisfied
+
+    # test errors
+    with pytest.raises(ValueError, match="One of the arguments is not an annotation"):
+        Collection(v1, 1, v2)
+    
+    with pytest.raises(TypeError, match="1 is not an annotation"):
+        a.add(1)