Merge pull request #614 from jhamrick/solution-region-delimeters

Don't be as strict about solution delimeters

nbgrader/preprocessors/clearsolutions.py

@@ -1,3 +1,5 @@
+import re
+
 from traitlets import Dict, Unicode, Bool, observe
 from textwrap import dedent
 
@@ -17,13 +19,13 @@ class ClearSolutions(NbGraderPreprocessor):
         help="The text snippet that will replace written solutions"
     ).tag(config=True)
 
-    begin_solution_delimeter = Dict(
-        dict(python="### BEGIN SOLUTION"),
+    begin_solution_delimeter = Unicode(
+        "BEGIN SOLUTION",
         help="The delimiter marking the beginning of a solution"
     ).tag(config=True)
 
-    end_solution_delimeter = Dict(
-        dict(python="### END SOLUTION"),
+    end_solution_delimeter = Unicode(
+        "END SOLUTION",
         help="The delimiter marking the end of a solution"
     ).tag(config=True)
 
@@ -44,28 +46,15 @@ class ClearSolutions(NbGraderPreprocessor):
     def _config_changed(self, change):
         new = change['new']
 
-        def check(x):
-            if x in new.ClearSolutions:
-                if not isinstance(new.ClearSolutions[x], dict):
-                    return True
-            return False
-
-        def fix(new, x):
-            self.log.warn(
-                "The ClearSolutions.{var} option must now be given as a "
-                "dictionary with keys for the language of the notebook. I will "
-                "automatically convert ClearSolutions.{var} to a dictionary "
-                "with a key for 'python', but note that this functionality may "
-                "be removed in future releases.".format(var=x))
-            new.ClearSolutions[x] = dict(python=new.ClearSolutions[x])
-            return new
-
-        if check('code_stub'):
-            fix(new, 'code_stub')
-        if check('begin_solution_delimeter'):
-            fix(new, 'begin_solution_delimeter')
-        if check('end_solution_delimeter'):
-            fix(new, 'end_solution_delimeter')
+        if 'code_stub' in new.ClearSolutions:
+            if not isinstance(new.ClearSolutions.code_stub, dict):
+                self.log.warn(
+                    "The ClearSolutions.code_stub option must now be given as a "
+                    "dictionary with keys for the language of the notebook. I will "
+                    "automatically convert ClearSolutions.code_stub to a dictionary "
+                    "with a key for 'python', but note that this functionality may "
+                    "be removed in future releases.")
+                new.ClearSolutions.code_stub = dict(python=new.ClearSolutions.code_stub)
 
         if 'comment_mark' in new.ClearSolutions:
             self.log.warn(
@@ -96,12 +85,10 @@ def _replace_solution_region(self, cell, language):
         new_lines = []
         in_solution = False
         replaced_solution = False
-        begin = self.begin_solution_delimeter[language]
-        end = self.end_solution_delimeter[language]
 
         for line in lines:
             # begin the solution area
-            if line.strip() == begin:
+            if self.begin_solution_delimeter in line:
 
                 # check to make sure this isn't a nested BEGIN
                 # SOLUTION region
@@ -113,12 +100,12 @@ def _replace_solution_region(self, cell, language):
                 replaced_solution = True
 
                 # replace it with the stub, indented as necessary
-                indent = line[:line.find(begin)]
+                indent = re.match(r"\s*", line).group(0)
                 for stub_line in stub_lines:
                     new_lines.append(indent + stub_line)
 
             # end the solution area
-            elif line.strip() == end:
+            elif self.end_solution_delimeter in line:
                 in_solution = False
 
             # add lines as long as it's not in the solution area
@@ -141,14 +128,6 @@ def preprocess(self, nb, resources):
             raise ValueError(
                 "language '{}' has not been specified in "
                 "ClearSolutions.code_stub".format(language))
-        if language not in self.begin_solution_delimeter:
-            raise ValueError(
-                "language '{}' has not been specified in "
-                "ClearSolutions.begin_solution_delimeter".format(language))
-        if language not in self.end_solution_delimeter:
-            raise ValueError(
-                "language '{}' has not been specified in "
-                "ClearSolutions.end_solution_delimeter".format(language))
 
         resources["language"] = language
         nb, resources = super(ClearSolutions, self).preprocess(nb, resources)

nbgrader/tests/preprocessors/test_clearsolutions.py

@@ -167,7 +167,25 @@ def test_preprocess_text_solution_cell_region(self, preprocessor):
         assert cell.source == "something something\nYOUR ANSWER HERE"
         assert cell.metadata.nbgrader['solution']
 
-    def test_preprocess_text_cell_region(self, preprocessor):
+    def test_preprocess_text_solution_cell_region_indented(self, preprocessor):
+        """Is a text grade cell correctly cleared and indented when there is a solution region?"""
+        cell = create_text_cell()
+        cell.source = dedent(
+            """
+            something something
+                ### BEGIN SOLUTION
+                this is the answer!
+                ### END SOLUTION
+            """
+        ).strip()
+        cell.metadata['nbgrader'] = dict(solution=True)
+        resources = dict(language="python")
+
+        cell = preprocessor.preprocess_cell(cell, resources, 1)[0]
+        assert cell.source == "something something\n    YOUR ANSWER HERE"
+        assert cell.metadata.nbgrader['solution']
+
+    def test_preprocess_text_cell_metadata(self, preprocessor):
         """Is an error thrown when a solution region exists in a non-solution text cell?"""
         cell = create_text_cell()
         cell.source = dedent(
@@ -205,12 +223,8 @@ def test_old_config(self):
         """Are deprecations handled cleanly?"""
         c = Config()
         c.ClearSolutions.code_stub = "foo"
-        c.ClearSolutions.begin_solution_delimeter = "bar"
-        c.ClearSolutions.end_solution_delimeter = "baz"
         pp = ClearSolutions(config=c)
         assert pp.code_stub == dict(python="foo")
-        assert pp.begin_solution_delimeter == dict(python="bar")
-        assert pp.end_solution_delimeter == dict(python="baz")
 
     def test_language_missing(self, preprocessor):
         nb = self._read_nb(os.path.join("files", "test.ipynb"))
@@ -221,12 +235,4 @@ def test_language_missing(self, preprocessor):
             preprocessor.preprocess(nb, {})
 
         preprocessor.code_stub = dict(javascript="foo")
-        with pytest.raises(ValueError):
-            preprocessor.preprocess(nb, {})
-
-        preprocessor.begin_solution_delimeter = dict(javascript="bar")
-        with pytest.raises(ValueError):
-            preprocessor.preprocess(nb, {})
-
-        preprocessor.end_solution_delimeter = dict(javascript="baz")
         preprocessor.preprocess(nb, {})