Repository: microsoft/pybryt

Issue:
Annotation for requiring/forbidding the use of specific functions
Instructors should be able to write annotations for asserting the use or non-use of specific functions.

This change is somewhat complicated, as it will involve editing the trace function to track information that it is not currently tracking (which function is being traced and to what package it belongs, namely).

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8a45ca0..4ddb41b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,11 @@
 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).
 
+## 0.1.6 - 2021-06-23
+
+* Added function call tracking to trace function and student implementations
+* Fixed bug for Markdown cells in `pybryt.StudentImplementation.errors`
+
 ## 0.1.5 - 2021-06-22
 
 * Added tracking and warnings for errors in student notebook execution
diff --git a/docs/student_implementations.rst b/docs/student_implementations.rst
index 0266b7d..4352852 100644
--- a/docs/student_implementations.rst
+++ b/docs/student_implementations.rst
@@ -23,7 +23,7 @@ The constructor reads the notebook file and stores the student's code. It then p
 the student's notebook using ``nbformat``'s ``ExecutePreprocessor``. The memory footprint of the
 student's code is constructed by executing the notebook with a trace function that tracks every 
 value created and accessed **by the student's code** and the timestamps at which those values were
-observed. 
+observed. PyBryt also tracks all of the function calls that occur during execution.
 
 To trace into code written in specific files, use the ``addl_filenames`` argument of the constructor 
 to pass a list of absolute paths of files to trace inside of. This can be useful for cases in which
diff --git a/pybryt/execution/__init__.py b/pybryt/execution/__init__.py
index 1c70931..4ab61aa 100644
--- a/pybryt/execution/__init__.py
+++ b/pybryt/execution/__init__.py
@@ -24,7 +24,8 @@ NBFORMAT_VERSION = 4
 
 
 def execute_notebook(nb: nbformat.NotebookNode, nb_path: str, addl_filenames: List[str] = [], 
-        output: Optional[str] = None) -> Tuple[int, List[Tuple[Any, int]], nbformat.NotebookNode]:
+        output: Optional[str] = None) -> Tuple[int, List[Tuple[Any, int]], List[Tuple[str, str]], \
+        nbformat.NotebookNode]:
     """
     Executes a submission using ``nbconvert`` and returns the memory footprint.
 
@@ -41,8 +42,9 @@ def execute_notebook(nb: nbformat.NotebookNode, nb_path: str, addl_filenames: Li
         output (``str``, optional): a file path at which to write the executed notebook
 
     Returns:
-        ``tuple[int, list[tuple[object, int]], nbformat.NotebookNode]``: the number of execution steps,
-        the memory footprint, and the executed notebook
+        ``tuple[int, list[tuple[object, int]], list[tuple[str, str]], nbformat.NotebookNode]``: the 
+        number of execution steps, the memory footprint, the list of function calls, and the 
+        executed notebook
     """
     nb = deepcopy(nb)
     preprocessor = IntermediateVariablePreprocessor()
@@ -55,7 +57,7 @@ def execute_notebook(nb: nbformat.NotebookNode, nb_path: str, addl_filenames: Li
     first_cell = nbformat.v4.new_code_cell(dedent(f"""\
         import sys
         from pybryt.execution import create_collector
-        observed_{secret}, cir = create_collector(addl_filenames={addl_filenames})
+        cir_results_{secret}, cir = create_collector(addl_filenames={addl_filenames})
         sys.settrace(cir)
         {TRACING_VARNAME} = True
         %cd {nb_dir}
@@ -65,9 +67,9 @@ def execute_notebook(nb: nbformat.NotebookNode, nb_path: str, addl_filenames: Li
         sys.settrace(None)
         import dill
         from pybryt.utils import filter_picklable_list
-        filter_picklable_list(observed_{secret})
+        filter_picklable_list(cir_results_{secret}[0])
         with open("{observed_fp}", "wb+") as f:
-            dill.dump(observed_{secret}, f)
+            dill.dump(cir_results_{secret}, f)
     """))
 
     nb['cells'].insert(0, first_cell)
@@ -82,10 +84,10 @@ def execute_notebook(nb: nbformat.NotebookNode, nb_path: str, addl_filenames: Li
             nbformat.write(nb, f)
 
     with open(observed_fp, "rb") as f:
-        observed = dill.load(f)
+        observed, calls = dill.load(f)
 
     os.remove(observed_fp)
 
     n_steps = max([t[1] for t in observed])
 
-    return n_steps, observed, nb
+    return n_steps, observed, calls, nb
diff --git a/pybryt/execution/tracing.py b/pybryt/execution/tracing.py
index 7aab9f4..03b44f3 100644
--- a/pybryt/execution/tracing.py
+++ b/pybryt/execution/tracing.py
@@ -17,7 +17,7 @@ TRACING_FUNC = None
 
 
 def create_collector(skip_types: List[type] = [type, type(len), ModuleType, FunctionType], addl_filenames: List[str] = []) -> \
-        Tuple[List[Tuple[Any, int]], Callable[[FrameType, str, Any], Callable]]:
+        Tuple[Tuple[List[Tuple[Any, int]], List[Tuple[str, str]]], Callable[[FrameType, str, Any], Callable]]:
     """
     Creates a list to collect observed values and a trace function.
 
@@ -32,11 +32,14 @@ def create_collector(skip_types: List[type] = [type, type(len), ModuleType, Func
             IPython
         
     Returns:
-        ``tuple[list[tuple[object, int]], callable[[frame, str, object], callable]]``: the list
-        of tuples of observed objects and their timestamps, and the trace function
+        ``tuple[tuple[list[tuple[object, int]], list[tuple[str, str]]], callable[[frame, str, 
+        object], callable]]``: a 2-tuple containing a 2-tuple with the list of tuples of observed 
+        objects and their timestamps and the list of call filenames andfunction names, and the trace 
+        function
     """
     global _COLLECTOR_RET
     observed = []
+    calls = []
     vars_not_found = {}
     hashes = set()
     counter = [0]
@@ -68,6 +71,15 @@ def create_collector(skip_types: List[type] = [type, type(len), ModuleType, Func
         except:
             return
 
+    def track_call(frame):
+        """
+        Tracks a call in ``calls`` as a tuple of ``(filename, function name)``.
+
+        Args:
+            frame (``types.FrameType``): the frame of the call
+        """
+        calls.append((frame.f_code.co_filename, frame.f_code.co_name))
+
     # TODO: a way to track the cell of execution
     def collect_intermidiate_results(frame: FrameType, event: str, arg: Any):
         """
@@ -76,6 +88,10 @@ def create_collector(skip_types: List[type] = [type, type(len), ModuleType, Func
         if frame.f_code.co_filename.startswith("<ipython") or frame.f_code.co_filename in addl_filenames:
             counter[0] += 1 # increment student code step counter
 
+        if event == "call":
+            track_call(frame)
+            return collect_intermidiate_results
+
         # return if tracking is disabled by a compelxity check
         from .complexity import _TRACKING_DISABLED
         if _TRACKING_DISABLED:
@@ -137,7 +153,7 @@ def create_collector(skip_types: List[type] = [type, type(len), ModuleType, Func
         return collect_intermidiate_results
 
     _COLLECTOR_RET = (observed, counter, collect_intermidiate_results)
-    return observed, collect_intermidiate_results
+    return (observed, calls), collect_intermidiate_results
 
 
 def _get_tracing_frame():
diff --git a/pybryt/student.py b/pybryt/student.py
index 996dbd9..456a839 100644
--- a/pybryt/student.py
+++ b/pybryt/student.py
@@ -49,6 +49,9 @@ class StudentImplementation(Serializable):
     values: List[Tuple[Any, int]]
     """the memory footprint (a list of tuples of objects and their timestamps)"""
 
+    calls: List[Tuple[str, str]]
+    """the list of all function calls from the student code"""
+
     steps: int
     """number of execution steps"""
 
@@ -59,7 +62,7 @@ class StudentImplementation(Serializable):
         self, path_or_nb: Optional[Union[str, nbformat.NotebookNode]], addl_filenames: List[str] = [],
         output: Optional[str] = None
     ):
-        self .executed_nb = None
+        self.executed_nb = None
         if path_or_nb is None:
             self.nb = None
             self.nb_path = None
@@ -84,7 +87,7 @@ class StudentImplementation(Serializable):
                 execution
             output (``str``, optional): a path at which to write executed notebook
         """
-        self.steps, self.values, self.executed_nb = execute_notebook(
+        self.steps, self.values, self.calls, self.executed_nb = execute_notebook(
             self.nb, self.nb_path, addl_filenames=addl_filenames, output=output
         )
 
@@ -105,14 +108,17 @@ class StudentImplementation(Serializable):
 
         errors = []
         for cell in self.executed_nb['cells']:
-            for out in cell['outputs']:
-                if out['output_type'] == "error":
-                    errors.append(out)
+            if cell['cell_type'] == "code":
+                for out in cell['outputs']:
+                    if out['output_type'] == "error":
+                        errors.append(out)
 
         return errors
 
     @classmethod
-    def from_footprint(cls, footprint: List[Tuple[Any, int]], steps: int) -> 'StudentImplementation':
+    def from_footprint(
+        cls, footprint: List[Tuple[Any, int]], calls: List[Tuple[str, str]], steps: int
+    ) -> 'StudentImplementation':
         """
         Create a student implementation object from a memory footprint directly, rather than by
         executing a notebook. Leaves the ``nb`` and ``nb_path`` instance variables of the resulting 
@@ -120,11 +126,13 @@ class StudentImplementation(Serializable):
 
         Args:
             footprint (``list[tuple[object, int]]``): the memory footprint
+            calls (``list[tuple[str, str]]``): the list of function calls
             steps (``int``): the number of execution steps
         """
         stu = cls(None)
         stu.steps = steps
         stu.values = footprint
+        stu.calls = calls
         return stu
 
     @classmethod
@@ -145,10 +153,12 @@ class StudentImplementation(Serializable):
         Returns:
             ``StudentImplementation``: the combined implementation
         """
-        new_mfp = []  # the new memory footprint
-        seen = set()  # set to track which values we've seen
+        new_mfp = []    # the new memory footprint
+        new_calls = []  # the new list of calls
+        seen = set()    # set to track which values we've seen
         timestamp_offset = 0  # offset for timestamps in the new memory footprint
         for impl in impls:
+            new_calls.extend(impl.calls)
             for obj, ts in impl.values:
                 h = pickle_and_hash(obj)
                 if h not in seen:
@@ -156,7 +166,7 @@ class StudentImplementation(Serializable):
                     new_mfp.append((obj, ts))
                     seen.add(h)
             timestamp_offset += impl.steps
-        return cls.from_footprint(new_mfp, timestamp_offset)
+        return cls.from_footprint(new_mfp, new_calls, timestamp_offset)
 
     @classmethod
     def from_cache(cls, cache_dir=CACHE_DIR_NAME, combine=True) -> \
@@ -191,7 +201,7 @@ class StudentImplementation(Serializable):
         footprint, the same number of steps, and the same source notebook.
         """
         return isinstance(other, type(self)) and self.values == other.values and \
-            self.steps == other.steps and self.nb == other.nb
+            self.steps == other.steps and self.nb == other.nb and self.calls == other.calls
 
     @property
     def _default_dump_dest(self) -> str:
@@ -337,6 +347,9 @@ class check:
     _observed: Optional[List[Tuple[Any, int]]]
     """the memory footprint"""
 
+    _calls: Optional[List[Tuple[str, str]]]
+    """the list of calls from tracing"""
+
     _cache: bool
     """whether to cache the memory footprint and results"""
 
@@ -366,6 +379,7 @@ class check:
         self._show_only = show_only
         self._frame = None
         self._observed = None
+        self._calls = None
         self._report_on_error = report_on_error
         self._cache = cache
 
@@ -396,7 +410,7 @@ class check:
             return  # if already tracing, no action required
 
         else:
-            self._observed, cir = create_collector(**self._kwargs)
+            (self._observed, self._calls), cir = create_collector(**self._kwargs)
             self._frame = inspect.currentframe().f_back
             self._frame.f_globals[TRACING_VARNAME] = True
 
@@ -409,7 +423,7 @@ class check:
             self._frame.f_globals[TRACING_VARNAME] = False
 
             if exc_type is None or self._report_on_error:
-                stu = StudentImplementation.from_footprint(self._observed, max(t[1] for t in self._observed))
+                stu = StudentImplementation.from_footprint(self._observed, self._calls, max(t[1] for t in self._observed))
                 res = stu.check(self._ref)
                 report = generate_report(res, show_only=self._show_only)
                 if report:


Test patch:
diff --git a/tests/execution/test_complexity.py b/tests/execution/test_complexity.py
index 90ab541..1074838 100644
--- a/tests/execution/test_complexity.py
+++ b/tests/execution/test_complexity.py
@@ -16,7 +16,7 @@ def test_time_complexity():
             2 * 2
         return n
 
-    observed, cir = create_collector()
+    (observed, _), cir = create_collector()
 
     for e in range(1, 9):
         n = 10 ** e
diff --git a/tests/execution/test_notebook_execution.py b/tests/execution/test_notebook_execution.py
index 7ce895a..2584552 100644
--- a/tests/execution/test_notebook_execution.py
+++ b/tests/execution/test_notebook_execution.py
@@ -42,6 +42,8 @@ def test_notebook_execution():
             with mock.patch("pybryt.execution.mkstemp") as mocked_tempfile:
                 mocked_tempfile.return_value = (None, observed_ntf.name)
 
-                n_steps, observed, _ = execute_notebook(nb, "", output=ntf.name)
+                n_steps, observed, calls, _ = execute_notebook(nb, "", output=ntf.name)
                 assert len(ntf.read()) > 0
                 assert n_steps == max(t[1] for t in observed)
+                assert isinstance(calls, list) and isinstance(calls[0], tuple) and \
+                    isinstance(calls[0][0], str) and isinstance(calls[0][1], str)
diff --git a/tests/execution/test_tracing.py b/tests/execution/test_tracing.py
index 46e722c..a8d26ba 100644
--- a/tests/execution/test_tracing.py
+++ b/tests/execution/test_tracing.py
@@ -23,7 +23,7 @@ def test_trace_function():
     tracked_filepath = "/path/to/tracked/file.py"
 
     frame = generate_mocked_frame("<ipython-abc123>", "foo", 3)
-    observed, cir = create_collector(addl_filenames=[tracked_filepath])
+    (observed, calls), cir = create_collector(addl_filenames=[tracked_filepath])
 
     arr = np.random.uniform(-100, 100, size=(100, 100))
     cir(frame, "return", arr)
@@ -42,6 +42,12 @@ def test_trace_function():
     
     assert len(observed) == 1
 
+    # test call event
+    assert len(calls) == 0
+    cir(frame, "call", None)
+    assert len(calls) == 1
+    assert calls[0] == ("<ipython-abc123>", "foo")
+
     frame = generate_mocked_frame(
         "<ipython-abc123>", "foo", 3, {"data": arr}
     )
@@ -54,7 +60,7 @@ def test_trace_function():
         cir(frame, "line", None)
         assert len(observed) == 2
         assert np.allclose(observed[1][0], arr.T)
-        assert observed[1][1] == 4
+        assert observed[1][1] == 5
 
         # check failed eval call for attributes
         mocked_linecache.return_value = "data.doesnt_exist"
@@ -68,7 +74,7 @@ def test_trace_function():
         cir(frame, "line", None)
         assert len(observed) == 3
         assert np.allclose(observed[2][0], frame.f_locals["more_data"])
-        assert observed[2][1] == 6
+        assert observed[2][1] == 7
 
         # check that we track assignment statements on function return
         mocked_linecache.return_value = "even_more_data = more_data ** 2"
@@ -85,11 +91,11 @@ def test_trace_function():
         cir(frame, "return", None)
         assert len(observed) == 6
         assert observed[3][0] is None
-        assert observed[3][1] == 9
+        assert observed[3][1] == 10
         assert np.allclose(observed[4][0], frame.f_locals["more_data"] ** 2)
-        assert observed[4][1] == 7
+        assert observed[4][1] == 8
         assert np.allclose(observed[5][0], frame.f_locals["more_data"] ** 3)
-        assert observed[5][1] == 8
+        assert observed[5][1] == 9
 
         # check that skip_types respected
         frame.f_locals["none_type"] = type(None)
@@ -106,7 +112,7 @@ def test_trace_function():
         cir(frame, "return", None) # run a return since arr shows up in vars_not_found
         assert len(observed) == 7
         assert np.allclose(observed[6][0], -1 * arr)
-        assert observed[6][1] == 12
+        assert observed[6][1] == 13
     
     # check that IPython child frame return values are tracked
     frame = generate_mocked_frame("/path/to/file.py", "bar", 100, f_back=frame)
@@ -116,7 +122,14 @@ def test_trace_function():
     cir(frame, "return", np.exp(arr))
     assert len(observed) == 8
     assert np.allclose(observed[7][0], np.exp(arr))
-    assert observed[7][1] == 12
+    assert observed[7][1] == 13
+
+    assert len(calls) == 1
+    frame = generate_mocked_frame("/path/to/foo.py", "bar", 100)
+    cir(frame, "call", None)
+    assert len(calls) == 2
+    assert calls[0] == ("<ipython-abc123>", "foo")
+    assert calls[1] == ("/path/to/foo.py", "bar")
 
 
 def test_tracing_control():
diff --git a/tests/test_student.py b/tests/test_student.py
index c94cd0f..9582f33 100644
--- a/tests/test_student.py
+++ b/tests/test_student.py
@@ -63,9 +63,14 @@ def test_constructor():
     assert stu.nb is nb
     assert stu.steps == max(t[1] for t in stu.values)
     assert len(stu.values) == 993
+    assert isinstance(stu.calls, list)
+    assert all(isinstance(c, tuple) for c in stu.calls)
+    assert all(len(c) == 2 for c in stu.calls)
+    assert all(isinstance(c[0], str) for c in stu.calls)
+    assert all(isinstance(c[1], str) for c in stu.calls)
 
     with mock.patch("pybryt.student.execute_notebook") as mocked_exec:
-        mocked_exec.return_value = (0, [], None)
+        mocked_exec.return_value = (0, [], [], None)
 
         with tempfile.NamedTemporaryFile(mode="w+", suffix=".ipynb") as ntf:
             nbformat.write(nb, ntf.name)
@@ -73,6 +78,7 @@ def test_constructor():
             stu = StudentImplementation(ntf.name)
             assert stu.steps == 0
             assert stu.values == []
+            assert stu.calls == []
             assert stu.nb == nb
 
     with pytest.raises(TypeError, match="path_or_nb is of unsupported type <class 'int'>"):
@@ -257,7 +263,7 @@ def test_generate_student_impls():
     nbs = [nb] * num_notebooks
 
     with mock.patch("pybryt.student.execute_notebook") as mocked_execute:
-        mocked_execute.return_value = (stu.steps, stu.values, None)
+        mocked_execute.return_value = (stu.steps, stu.values, stu.calls, None)
         stus = generate_student_impls(nbs)
 
     assert all(s == stu for s in stus)