From 41a85f9d14d3637dbd45214755c5612f096552b6 Mon Sep 17 00:00:00 2001 From: Martin Olveyra Date: Tue, 26 Oct 2010 16:11:04 -0200 Subject: [PATCH] Added support for variants when applied to tag attributes. fixed handling of variants with single attribute. Removed unneeded object attribute surrounds_variant. Added new tests cases for fixes. --- scrapy/contrib/ibl/extraction/pageobjects.py | 5 +- scrapy/contrib/ibl/extraction/pageparsing.py | 21 +-- .../contrib/ibl/extraction/regionextract.py | 5 +- .../tests/test_contrib_ibl/test_extraction.py | 144 ++++++++++++++++++ .../test_contrib_ibl/test_pageparsing.py | 50 +++++- 5 files changed, 209 insertions(+), 16 deletions(-) diff --git a/scrapy/contrib/ibl/extraction/pageobjects.py b/scrapy/contrib/ibl/extraction/pageobjects.py index 404106674..e73b4e699 100644 --- a/scrapy/contrib/ibl/extraction/pageobjects.py +++ b/scrapy/contrib/ibl/extraction/pageobjects.py @@ -194,18 +194,17 @@ class AnnotationTag(object): """ __slots__ = ('surrounds_attribute', 'start_index', 'end_index', 'tag_attributes', 'annotation_text', 'variant_id', - 'surrounds_variant','match_common_prefix', 'metadata') + 'match_common_prefix', 'metadata') def __init__(self, start_index, end_index, surrounds_attribute=None, annotation_text=None, tag_attributes=None, variant_id=None, - surrounds_variant=None, match_common_prefix=False): + match_common_prefix=False): self.start_index = start_index self.end_index = end_index self.surrounds_attribute = surrounds_attribute self.annotation_text = annotation_text self.tag_attributes = tag_attributes or [] self.variant_id = variant_id - self.surrounds_variant = surrounds_variant self.match_common_prefix = match_common_prefix self.metadata = {} diff --git a/scrapy/contrib/ibl/extraction/pageparsing.py b/scrapy/contrib/ibl/extraction/pageparsing.py index df9b5c184..b672ef502 100644 --- a/scrapy/contrib/ibl/extraction/pageparsing.py +++ b/scrapy/contrib/ibl/extraction/pageparsing.py @@ -201,29 +201,32 @@ class TemplatePageParser(InstanceLearningParser): self.extra_required_attrs.extend(jannotation.pop('required', [])) - variant_id = jannotation.pop('variant', 0) - if variant_id > 0: - self.variant_stack.append(variant_id) - annotation.surrounds_variant = variant_id attribute_annotations = jannotation.pop('annotations', {}).items() for extract_attribute, tag_value in attribute_annotations: if extract_attribute == 'content': annotation.surrounds_attribute = tag_value else: annotation.tag_attributes.append((extract_attribute, tag_value)) - + + variant_id = jannotation.pop('variant', 0) + if variant_id > 0: + if annotation.surrounds_attribute is not None: + self.variant_stack.append(variant_id) + else: + annotation.variant_id = variant_id + annotation.metadata = jannotation if annotation.annotation_text is None: self.next_tag_index += 1 - if self.variant_stack: + if self.variant_stack and annotation.variant_id is None: variant_id = self.variant_stack[-1] if variant_id == '0': variant_id = None annotation.variant_id = variant_id # look for a closing tag if the content is important - if annotation.surrounds_attribute or annotation.surrounds_variant: + if annotation.surrounds_attribute: self.labelled_tag_stacks[html_tag.tag].append(annotation) else: annotation.end_index = annotation.start_index + 1 @@ -272,9 +275,9 @@ class TemplatePageParser(InstanceLearningParser): self.next_tag_index += 1 if len(labelled_tags) == 0: del self.labelled_tag_stacks[html_tag.tag] - if annotation.surrounds_variant and self.variant_stack: + if annotation.variant_id and self.variant_stack: prev = self.variant_stack.pop() - if prev != annotation.surrounds_variant: + if prev != annotation.variant_id: raise ValueError("unbalanced variant annotation tags") def handle_data(self, html_data_fragment): diff --git a/scrapy/contrib/ibl/extraction/regionextract.py b/scrapy/contrib/ibl/extraction/regionextract.py index e84c82cd3..fc5470f5b 100644 --- a/scrapy/contrib/ibl/extraction/regionextract.py +++ b/scrapy/contrib/ibl/extraction/regionextract.py @@ -25,7 +25,7 @@ def build_extraction_tree(template, type_descriptor, trace=True): extractors = BasicTypeExtractor.create(template.annotations, attribute_map) if trace: extractors = TraceExtractor.apply(template, extractors) - for cls in (RepeatedDataExtractor, AdjacentVariantExtractor, RepeatedDataExtractor, + for cls in (AdjacentVariantExtractor, RepeatedDataExtractor, AdjacentVariantExtractor, RepeatedDataExtractor, RecordExtractor): extractors = cls.apply(template, extractors) if trace: @@ -465,9 +465,8 @@ class AdjacentVariantExtractor(RecordExtractor): continue if vid in adjacent_variants: adjacent_variants.remove(vid) - elif len(list(egroup)) > 1: + else: adjacent_variants.add(vid) - new_extractors = [] for variant, group_seq in groupby(extractors, variantf): group_seq = list(group_seq) diff --git a/scrapy/tests/test_contrib_ibl/test_extraction.py b/scrapy/tests/test_contrib_ibl/test_extraction.py index 2c5cbb30a..895e563b7 100644 --- a/scrapy/tests/test_contrib_ibl/test_extraction.py +++ b/scrapy/tests/test_contrib_ibl/test_extraction.py @@ -633,6 +633,121 @@ EXTRACT_PAGE20 = u""" """ +ANNOTATED_PAGE21 = u""" + + +

+ + + + + + + + + + +
+ +

tables
+ + +""" + +EXTRACT_PAGE21 = u""" + + +

+ + + + + + + + + + +
+ +

chairs
+ +""" + +ANNOTATED_PAGE22 = u""" + + +

+ + + + + + + + + + +
+

product 1

+$67 + +
+

product 2

+$70 + +
+

product 3

+$73 + +
+

product 4

+$80 + +
+ +

tables
+ + +""" + +EXTRACT_PAGE22 = u""" + + +

+ + + + + + + + + + +
+

product 1

+$70 + +
+

product 2

+$80 + +
+

product 3

+$90 + +
+

product 4

+$100 + +
+ +

chairs
+ +""" + + SAMPLE_DESCRIPTOR1 = ItemDescriptor('test', 'product test', [ A('name', "Product name", required=True), A('price', "Product price, including any discounts and tax or vat", @@ -813,6 +928,35 @@ TEST_DATA = [ {'price': ['330'], 'name': ['Queen']}, ]}, ), + ('variants with swatches', [ANNOTATED_PAGE21], EXTRACT_PAGE21, None, + {u'category': [u'chairs'], + u'image_urls': [u'image.jpg'], + u'variants': [ + {'swatches': ['swatch1.jpg']}, + {'swatches': ['swatch2.jpg']}, + {'swatches': ['swatch3.jpg']}, + {'swatches': ['swatch4.jpg']}, + ] + }, + ), + ('variants with swatches complete', [ANNOTATED_PAGE22], EXTRACT_PAGE22, None, + {u'category': [u'chairs'], + u'variants': [ + {u'swatches': [u'swatch1.jpg'], + u'price': [u'$70'], + u'name': [u'product 1']}, + {u'swatches': [u'swatch2.jpg'],\ + u'price': [u'$80'], + u'name': [u'product 2']}, + {u'swatches': [u'swatch3.jpg'], + u'price': [u'$90'], + u'name': [u'product 3']}, + {u'swatches': [u'swatch4.jpg'], + u'price': [u'$100'], + u'name': [u'product 4']} + ], + u'image_urls': [u'image.jpg']}, + ), ] class TestExtraction(TestCase): diff --git a/scrapy/tests/test_contrib_ibl/test_pageparsing.py b/scrapy/tests/test_contrib_ibl/test_pageparsing.py index ae5e1c129..8b88c81d8 100644 --- a/scrapy/tests/test_contrib_ibl/test_pageparsing.py +++ b/scrapy/tests/test_contrib_ibl/test_pageparsing.py @@ -155,6 +155,32 @@ Description """ +LABELLED_PAGE9 = u""" + + +

product 1

+$67 +

product 2

+$70 +
tables
+ +""" + +LABELLED_PAGE10 = u""" + + +

product 1

+$67 + + +

product 2

+$70 + + +
tables
+ +""" + def _parse_page(parser_class, pagetext): htmlpage = HtmlPage(None, {}, pagetext) parser = parser_class(TokenDict()) @@ -268,7 +294,29 @@ class TestPageParsing(TestCase): """Test parsing of extra required attributes""" p = _parse_page(TemplatePageParser, LABELLED_PAGE8) self.assertEqual(p.extra_required_attrs, ["description"]) - + + def test_variants(self): + """Test parsing of variant annotations""" + annotations = _parse_page(TemplatePageParser, LABELLED_PAGE9).annotations + self.assertEqual(annotations[0].variant_id, None) + self.assertEqual(annotations[1].variant_id, 1) + self.assertEqual(annotations[2].variant_id, 1) + self.assertEqual(annotations[3].variant_id, 2) + self.assertEqual(annotations[4].variant_id, 2) + self.assertEqual(annotations[5].variant_id, None) + + def test_variants_in_attributes(self): + """Test parsing of variant annotations in attributes""" + annotations = _parse_page(TemplatePageParser, LABELLED_PAGE10).annotations + self.assertEqual(annotations[0].variant_id, None) + self.assertEqual(annotations[1].variant_id, 1) + self.assertEqual(annotations[2].variant_id, 1) + self.assertEqual(annotations[3].variant_id, 1) + self.assertEqual(annotations[4].variant_id, 2) + self.assertEqual(annotations[5].variant_id, 2) + self.assertEqual(annotations[6].variant_id, 2) + self.assertEqual(annotations[7].variant_id, None) + def test_site_pages(self): """ Tests from real pages. More reliable and easy to build for more complicated structures