Merge pull request #37 from kalessin/replacement

correctly handle tag name replacement when replaced tags are not closed.

Author: shaneaevans

scrapely/extraction/pageparsing.py

@@ -66,7 +66,14 @@ def handle_tag(self, html_tag, index):
         pass
 
 _END_UNPAIREDTAG_TAGS = ["form", "div", "p", "table", "tr", "td"]
-
+_AUTO_CLOSE_TAGS_ON_OPEN = {
+    # the given keys closes the tags in the list
+    "p": ["p"],
+    "option": ["option"],
+}
+_AUTO_CLOSE_TAGS_ON_CLOSE = {
+    "select": ["option"],
+}
 class TemplatePageParser(InstanceLearningParser):
     """Template parsing for instance based learning algorithm"""
 
@@ -169,19 +176,15 @@ def _handle_open_tag(self, html_tag):
                 self._close_unpaired_tag()
             else:
                 self.unpairedtag_stack.append(html_tag.tag)
-            
-        # can't be a p inside another p. Also, an open p element closes
-        # a previous open p element.
-        if html_tag.tag == "p" and html_tag.tag in self.labelled_tag_stacks:
-            annotation = self.labelled_tag_stacks.pop(html_tag.tag)[0]
-            annotation.end_index = self.next_tag_index
-            self.annotations.append(annotation)
-                
+        
+        tagname = replacement or self._update_replacement_stack(html_tag)
+        self._handle_unclosed_tags(tagname, _AUTO_CLOSE_TAGS_ON_OPEN)
+               
         jannotation = self._read_template_annotation(html_tag)
         if not jannotation:
-            if html_tag.tag in self.labelled_tag_stacks:
+            if tagname in self.labelled_tag_stacks:
                 # add this tag to the stack to match correct end tag
-                self.labelled_tag_stacks[html_tag.tag].append(None)
+                self.labelled_tag_stacks[tagname].append(None)
             self.next_tag_index += 1
             return
 
@@ -227,7 +230,7 @@ def _handle_open_tag(self, html_tag):
 
         # look for a closing tag if the content is important
         if annotation.surrounds_attribute:
-            self.labelled_tag_stacks[html_tag.tag].append(annotation)
+            self.labelled_tag_stacks[tagname].append(annotation)
         else:
             annotation.end_index = annotation.start_index + 1
             self.annotations.append(annotation)
@@ -239,6 +242,7 @@ def _handle_close_tag(self, html_tag):
                 self.unpairedtag_stack.pop()
             else:
                 self._close_unpaired_tag()
+
         ignored_tags = self.ignored_tag_stacks.get(html_tag.tag)
         if ignored_tags is not None:
             tag = ignored_tags.pop()
@@ -250,15 +254,10 @@ def _handle_close_tag(self, html_tag):
             if len(ignored_tags) == 0:
                 del self.ignored_tag_stacks[html_tag.tag]
 
-        if html_tag.tag in self.replacement_stacks:
-            replacement = self.replacement_stacks[html_tag.tag].pop()
-            if replacement:
-                self.token_list.pop()
-                self._add_token(replacement, html_tag.tag_type, html_tag.start, html_tag.end)
-            if len(self.replacement_stacks[html_tag.tag]) == 0:
-                del self.replacement_stacks[html_tag.tag]
+        tagname = self._update_replacement_stack(html_tag)
+        self._handle_unclosed_tags(tagname, _AUTO_CLOSE_TAGS_ON_CLOSE)
 
-        labelled_tags = self.labelled_tag_stacks.get(html_tag.tag)
+        labelled_tags = self.labelled_tag_stacks.get(tagname)
         if labelled_tags is None:
             self.next_tag_index += 1
             return
@@ -274,12 +273,35 @@ def _handle_close_tag(self, html_tag):
             else:
                 self.next_tag_index += 1
             if len(labelled_tags) == 0:
-                del self.labelled_tag_stacks[html_tag.tag]
+                del self.labelled_tag_stacks[tagname]
             if annotation.variant_id and self.variant_stack:
                 prev = self.variant_stack.pop()
                 if prev != annotation.variant_id:
                     raise ValueError("unbalanced variant annotation tags")
 
+    def _update_replacement_stack(self, html_tag):
+        replacement = html_tag.tag
+        if html_tag.tag in self.replacement_stacks:
+            replacement = self.replacement_stacks[html_tag.tag].pop()
+            if replacement:
+                self.token_list.pop()
+                self._add_token(replacement, html_tag.tag_type, html_tag.start, html_tag.end)
+            if len(self.replacement_stacks[html_tag.tag]) == 0:
+                del self.replacement_stacks[html_tag.tag]
+        return replacement
+
+    def _handle_unclosed_tags(self, tagname, auto_close_tags):
+        """I.e. can't be a p inside another p. Also, an open p element closes
+        a previous open p element"""
+        if tagname in auto_close_tags:
+            for _close_tag in auto_close_tags[tagname]:
+                if _close_tag in self.labelled_tag_stacks:
+                    annotation = self.labelled_tag_stacks.pop(_close_tag)[0]
+                    annotation.end_index = self.next_tag_index
+                    self.annotations.append(annotation)
+                    break
+        return tagname
+
     def handle_data(self, html_data_fragment, index):
         fragment_text = self.html_page.fragment_data(html_data_fragment)
         self._process_text(fragment_text)

scrapely/tests/test_pageparsing.py

@@ -117,6 +117,20 @@
 </body></html>
 """
 
+LABELLED_PAGE5a = u"""
+<ul data-scrapy-replacement="select" name="txtvariant" class="smalltextblk">
+<li data-scrapy-replacement="option" data-scrapy-annotate="{&quot;required&quot;: [], &quot;variant&quot;: 0, &quot;annotations&quot;: {&quot;content&quot;: &quot;price&quot;}, &quot;generated&quot;: false}" value="BLUE">Blue&nbsp;$9.95 - In Stock</li> 
+<li data-scrapy-replacement="option" data-scrapy-annotate="{&quot;required&quot;: [], &quot;variant&quot;: 0, &quot;annotations&quot;: {&quot;content&quot;: &quot;price&quot;}, &quot;generated&quot;: false}" value="RED">Red&nbsp;$9.95 - In Stock</li>
+</ul>
+"""
+
+LABELLED_PAGE5b = u"""
+<ul data-scrapy-replacement="select" name="txtvariant" class="smalltextblk">
+<li data-scrapy-replacement="option" data-scrapy-annotate="{&quot;required&quot;: [], &quot;variant&quot;: 0, &quot;annotations&quot;: {&quot;content&quot;: &quot;price&quot;}, &quot;generated&quot;: false}" value="BLUE">Blue&nbsp;$9.95 - In Stock
+<li data-scrapy-replacement="option" data-scrapy-annotate="{&quot;required&quot;: [], &quot;variant&quot;: 0, &quot;annotations&quot;: {&quot;content&quot;: &quot;price&quot;}, &quot;generated&quot;: false}" value="RED">Red&nbsp;$9.95 - In Stock
+</ul>
+"""
+
 LABELLED_PAGE6 = u"""
 <html><body>
 Text A
@@ -266,6 +280,29 @@ def test_replacement(self):
         p = _parse_page(TemplatePageParser, LABELLED_PAGE5)
         self.assertEqual(_tags(p, bool), ['<html>', '<body>', '<select>', '<option>',
                     '</option>', '<li>', '</li>', '<option>', '</option>', '</select>', '</body>', '</html>'])
+
+    def test_replacement2(self):
+        """Replacement, with annotations"""
+        p = _parse_page(TemplatePageParser, LABELLED_PAGE5a)
+        self.assertEqual(_tags(p, bool), [u'<select>', u'<option>', u'</option>', u'<option>', u'</option>', u'</select>'])
+        self.assertEqual(p.annotations[0].surrounds_attribute, 'price')
+        self.assertEqual(p.annotations[0].start_index, 1)
+        self.assertEqual(p.annotations[0].end_index, 2)
+        self.assertEqual(p.annotations[1].surrounds_attribute, 'price')
+        self.assertEqual(p.annotations[1].start_index, 3)
+        self.assertEqual(p.annotations[1].end_index, 4)
+
+
+    def test_replacement3(self):
+        """A second case of replacement, with annotations, not closed replaced tags"""
+        p = _parse_page(TemplatePageParser, LABELLED_PAGE5b)
+        self.assertEqual(_tags(p, bool), [u'<select>', u'<option>', u'<option>', u'</select>'])
+        self.assertEqual(p.annotations[0].surrounds_attribute, 'price')
+        self.assertEqual(p.annotations[0].start_index, 1)
+        self.assertEqual(p.annotations[0].end_index, 2)
+        self.assertEqual(p.annotations[1].surrounds_attribute, 'price')
+        self.assertEqual(p.annotations[1].start_index, 2)
+        self.assertEqual(p.annotations[1].end_index, 3)
 
     def test_partial(self):
         """Test partial annotation parsing"""