From 7bc7af0162225e8431e8f5cfaf3eae9fbd099a6d Mon Sep 17 00:00:00 2001 From: Pablo Hoffman Date: Wed, 5 Aug 2009 00:41:02 -0300 Subject: [PATCH] ItemLoader: some more code cleanups, and added many more tests --- scrapy/newitem/__init__.py | 10 +- scrapy/newitem/loader/__init__.py | 37 +++--- scrapy/newitem/loader/expanders.py | 79 +++++++------ scrapy/newitem/loader/reducers.py | 37 ++++-- scrapy/tests/test_itemloader.py | 177 ++++++++++++++++++----------- 5 files changed, 197 insertions(+), 143 deletions(-) diff --git a/scrapy/newitem/__init__.py b/scrapy/newitem/__init__.py index e32dd9660..2d301826b 100644 --- a/scrapy/newitem/__init__.py +++ b/scrapy/newitem/__init__.py @@ -3,7 +3,7 @@ from UserDict import DictMixin from scrapy.item.models import BaseItem class Field(dict): - pass + """Container of field metadata""" class _ItemMeta(type): @@ -30,8 +30,7 @@ class Item(DictMixin, BaseItem): def __init__(self, *args, **kwargs): self._values = {} - - if args or kwargs: # don't instantiate dict for simple (most common) case + if args or kwargs: # avoid instantiating dict for most common case for k, v in dict(*args, **kwargs).iteritems(): self[k] = v @@ -70,8 +69,3 @@ class Item(DictMixin, BaseItem): """ values = ', '.join('%s=%r' % field for field in self.iteritems()) return "%s(%s)" % (self.__class__.__name__, values) - - def get_id(self): - """Returns the unique id for this item.""" - raise NotImplementedError - diff --git a/scrapy/newitem/loader/__init__.py b/scrapy/newitem/loader/__init__.py index 2ff6ecfd9..b5002c05f 100644 --- a/scrapy/newitem/loader/__init__.py +++ b/scrapy/newitem/loader/__init__.py @@ -1,15 +1,19 @@ from collections import defaultdict +from scrapy.utils.datatypes import MergeDict from scrapy.newitem import Item -from scrapy.newitem.loader.reducers import take_first +from scrapy.newitem.loader.reducers import TakeFirst +from scrapy.newitem.loader.expanders import IdentityExpander class ItemLoader(object): - item_class = Item + default_item_class = Item + default_expander = IdentityExpander() + default_reducer = TakeFirst() def __init__(self, **loader_args): self._response = loader_args.get('response') - self._item = loader_args.get('item') or self.item_class() + self._item = loader_args.setdefault('item', self.default_item_class()) self._loader_args = loader_args self._values = defaultdict(list) @@ -35,25 +39,22 @@ class ItemLoader(object): return reducer(self._values[field_name]) def get_expander(self, field_name): - return getattr(self, 'expand_%s' % field_name, self.expand) + expander = getattr(self, '%s_exp' % field_name, None) + if not expander: + expander = self._item.fields[field_name].get('expander', \ + self.default_expander) + return expander def get_reducer(self, field_name): - try: - return getattr(self, 'reduce_%s' % field_name) - except AttributeError: - return self._item.fields[field_name].get('reducer', self.reduce) + reducer = getattr(self, '%s_red' % field_name, None) + if not reducer: + reducer = self._item.fields[field_name].get('reducer', \ + self.default_reducer) + return reducer def _expand_value(self, field_name, value, new_loader_args): + loader_args = self._loader_args if new_loader_args: - loader_args = self._loader_args.copy() - loader_args.update(new_loader_args) - else: # shortcut for most common case - loader_args = self._loader_args + loader_args = MergeDict(new_loader_args, self._loader_args) expander = self.get_expander(field_name) return expander(value, loader_args=loader_args) - - def expand(self, value, loader_args): # default expander - return value - - def reduce(self, values): # default reducer - return take_first(values) diff --git a/scrapy/newitem/loader/expanders.py b/scrapy/newitem/loader/expanders.py index fbab377d4..33fc0f833 100644 --- a/scrapy/newitem/loader/expanders.py +++ b/scrapy/newitem/loader/expanders.py @@ -1,62 +1,59 @@ """ -ItemLoader expanders +This module provides some commonly used Expanders """ -from types import UnboundMethodType - from scrapy.utils.misc import arg_to_iter from scrapy.utils.python import get_func_args from scrapy.utils.datatypes import MergeDict -def tree_expander(*functions, **default_loader_args): - """Create an ItemLoader expander from a list of functions using the tree - expansion algorithm described below. +class TreeExpander(object): + """An expander which applies the given list of functions consecutively to + each value returned by the previous function. - The functions can optionally accept a ``loader_args`` argument which (if - present) will be used to pass loader arguments when the function is called. - - The tree expansion algorithm consists in an ordered list of functions, each - of which receives one value and can return zero, one or more values (as a - list or iterable). If a function returns more than one value, the next - function in the pipeline will be called with each of those values, - potentially returning more values. Hence the name "tree expansion - algorithm". + The algorithm consists in an ordered list of functions, each of which + receives one value and can return zero, one or more values (as a list or + iterable). If a function returns more than one value, the next function in + the list will be called with each of those values, potentially returning + more values and thus expanding the execution into different branches, which + is why this expander is called Tree Expander. + + The expander functions can optionally receive a ``loader_args`` argument, + which will contain the current active loader arguments. """ - def wrap_y(f): - return lambda x, y, z: f(y) - def wrap_xy(f): - return lambda x, y, z: f(x, y) - - def wrap_yz(f): - return lambda x, y, z: f(y, z) - - wrapped_funcs = [] - for func in functions: - if isinstance(func, UnboundMethodType): - func = func.im_func + def __init__(self, *functions, **default_loader_args): + self.default_loader_args = default_loader_args + self.wrapped_funcs = [] + for func in functions: if 'loader_args' in get_func_args(func): - wfunc = func + wfunc = self.wrap_with_args(func) else: - wfunc = wrap_xy(func) - else: - if 'loader_args' in get_func_args(func): - wfunc = wrap_yz(func) - else: - wfunc = wrap_y(func) - wrapped_funcs.append(wfunc) + wfunc = self.wrap_no_args(func) + self.wrapped_funcs.append(wfunc) + + def wrap_no_args(self, f): + return lambda x, y: f(x) - def _expander(loader, value, loader_args): + def wrap_with_args(self, f): + return lambda x, y: f(x, loader_args=y) + + def __call__(self, value, loader_args): values = arg_to_iter(value) - largs = default_loader_args + largs = self.default_loader_args if loader_args: - largs = MergeDict(loader_args, default_loader_args) - for func in wrapped_funcs: + largs = MergeDict(loader_args, self.default_loader_args) + for func in self.wrapped_funcs: next_values = [] for v in values: - next_values += arg_to_iter(func(loader, v, largs)) + next_values += arg_to_iter(func(v, largs)) values = next_values return list(values) - return _expander +class IdentityExpander(object): + """An expander which returns the original values unchanged. It doesn't + support any constructor arguments. + """ + + def __call__(self, values, loader_args): + return arg_to_iter(values) diff --git a/scrapy/newitem/loader/reducers.py b/scrapy/newitem/loader/reducers.py index f0b78cb21..76ba754b4 100644 --- a/scrapy/newitem/loader/reducers.py +++ b/scrapy/newitem/loader/reducers.py @@ -1,14 +1,33 @@ """ -ItemLoader reducers +This module provides some commonly used Reducers """ -def take_first(values): - for value in values: - if value: - return value +class TakeFirst(object): + """Return the first non-null value from the list to reduce""" -def identity(values): - return values + def __call__(self, values): + for value in values: + if value: + return value -def join_strings(values): - return u' '.join(values) + +class Identity(object): + """Return the list to reduce untouched""" + + def __call__(self, values): + return values + + +class JoinStrings(object): + """Return a string with the contents of the list to reduce joined with the + separator given in the constructor, which defaults to u' '. + + When using the default separator, this reducer is equivalent to the + function: u' '.join + """ + + def __init__(self, separator=u' '): + self.separator = separator + + def __call__(self, values): + return self.separator.join(values) diff --git a/scrapy/tests/test_itemloader.py b/scrapy/tests/test_itemloader.py index fa5d777c9..484257e67 100644 --- a/scrapy/tests/test_itemloader.py +++ b/scrapy/tests/test_itemloader.py @@ -1,175 +1,218 @@ import unittest from scrapy.newitem.loader import ItemLoader -from scrapy.newitem.loader.expanders import tree_expander +from scrapy.newitem.loader.expanders import TreeExpander, IdentityExpander +from scrapy.newitem.loader.reducers import JoinStrings, Identity from scrapy.newitem import Item, Field +# test items -class BaseItem(Item): +class NameItem(Item): name = Field() - -class TestItem(BaseItem): +class TestItem(NameItem): url = Field() summary = Field() +# test loaders -class BaseItemLoader(ItemLoader): - item_class = TestItem +class NameItemLoader(ItemLoader): + default_item_class = TestItem +class TestItemLoader(NameItemLoader): + name_exp = TreeExpander(lambda v: v.title()) -class TestItemLoader(BaseItemLoader): - expand_name = tree_expander(lambda v: v.title()) +class DefaultedItemLoader(NameItemLoader): + default_expander = TreeExpander(lambda v: v[:-1]) +# test expanders -class DefaultedItemLoader(BaseItemLoader): - expand = tree_expander(lambda v: v[:-1]) - - -class InheritDefaultedItemLoader(DefaultedItemLoader): - pass - +def expander_func_with_args(value, other=None, loader_args=None): + if 'key' in loader_args: + return loader_args['key'] + return value class ItemLoaderTest(unittest.TestCase): - def test_get_item(self): - il = TestItemLoader() + def test_get_item_using_default_loader(self): + i = TestItem() + i['summary'] = u'lala' + il = ItemLoader(item=i) + il.add_value('name', u'marta') + item = il.get_item() + assert item is i + self.assertEqual(item['summary'], u'lala') + self.assertEqual(item['name'], u'marta') + def test_get_item_using_custom_loader(self): + il = TestItemLoader() il.add_value('name', u'marta') item = il.get_item() self.assertEqual(item['name'], u'Marta') def test_add_value(self): il = TestItemLoader() - il.add_value('name', u'marta') self.assertEqual(il.get_expanded_value('name'), [u'Marta']) self.assertEqual(il.get_reduced_value('name'), u'Marta') - il.add_value('name', u'pepe') self.assertEqual(il.get_expanded_value('name'), [u'Marta', u'Pepe']) self.assertEqual(il.get_reduced_value('name'), u'Marta') def test_replace_value(self): il = TestItemLoader() - il.replace_value('name', u'marta') self.assertEqual(il.get_expanded_value('name'), [u'Marta']) self.assertEqual(il.get_reduced_value('name'), u'Marta') - il.replace_value('name', u'pepe') self.assertEqual(il.get_expanded_value('name'), [u'Pepe']) self.assertEqual(il.get_reduced_value('name'), u'Pepe') - def test_multiple_functions(self): - class TestItemLoader(BaseItemLoader): - expand_name = tree_expander(lambda v: v.title(), lambda v: v[:-1]) + def test_tree_expander_multiple_functions(self): + class TestItemLoader(NameItemLoader): + name_exp = TreeExpander(lambda v: v.title(), lambda v: v[:-1]) il = TestItemLoader() - il.add_value('name', u'marta') self.assertEqual(il.get_reduced_value('name'), u'Mart') - item = il.get_item() self.assertEqual(item['name'], u'Mart') - def test_defaulted(self): - dil = DefaultedItemLoader() + def test_default_expander(self): + il = DefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_reduced_value('name'), u'mart') - dil.add_value('name', u'marta') - self.assertEqual(dil.get_reduced_value('name'), u'mart') + def test_inherited_default_expander(self): + class InheritDefaultedItemLoader(DefaultedItemLoader): + pass - def test_inherited_default(self): - dil = InheritDefaultedItemLoader() + il = InheritDefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_reduced_value('name'), u'mart') - dil.add_value('name', u'marta') - self.assertEqual(dil.get_reduced_value('name'), u'mart') - - def test_inheritance(self): + def test_expander_inheritance(self): class ChildItemLoader(TestItemLoader): - expand_url = tree_expander(lambda v: v.lower()) + url_exp = TreeExpander(lambda v: v.lower()) il = ChildItemLoader() - il.add_value('url', u'HTTP://scrapy.ORG') self.assertEqual(il.get_reduced_value('url'), u'http://scrapy.org') - il.add_value('name', u'marta') self.assertEqual(il.get_reduced_value('name'), u'Marta') class ChildChildItemLoader(ChildItemLoader): - expand_url = tree_expander(lambda v: v.upper()) - expand_summary = tree_expander(lambda v: v) + url_exp = TreeExpander(lambda v: v.upper()) + summary_exp = TreeExpander(lambda v: v) il = ChildChildItemLoader() - il.add_value('url', u'http://scrapy.org') self.assertEqual(il.get_reduced_value('url'), u'HTTP://SCRAPY.ORG') - il.add_value('name', u'marta') self.assertEqual(il.get_reduced_value('name'), u'Marta') - def test_identity(self): + def test_empty_tree_expander(self): class IdentityDefaultedItemLoader(DefaultedItemLoader): - expand_name = tree_expander() + name_exp = TreeExpander() il = IdentityDefaultedItemLoader() - il.add_value('name', u'marta') self.assertEqual(il.get_reduced_value('name'), u'marta') - def test_staticmethods(self): + def test_identity_expander(self): + class IdentityDefaultedItemLoader(DefaultedItemLoader): + name_exp = IdentityExpander() + + il = IdentityDefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_reduced_value('name'), u'marta') + + def test_extend_expanders(self): class ChildItemLoader(TestItemLoader): - expand_name = tree_expander(TestItemLoader.expand_name, unicode.swapcase) + name_exp = TreeExpander(TestItemLoader.name_exp, unicode.swapcase) il = ChildItemLoader() - il.add_value('name', u'marta') self.assertEqual(il.get_reduced_value('name'), u'mARTA') - def test_staticdefaults(self): class ChildDefaultedItemLoader(DefaultedItemLoader): - expand_name = tree_expander(DefaultedItemLoader.expand, unicode.swapcase) + name_exp = TreeExpander(DefaultedItemLoader.default_expander, unicode.swapcase) il = ChildDefaultedItemLoader() - il.add_value('name', u'marta') self.assertEqual(il.get_reduced_value('name'), u'MART') - def test_reducer(self): + def test_reducer_using_function(self): il = TestItemLoader() - il.add_value('name', [u'mar', u'ta']) self.assertEqual(il.get_reduced_value('name'), u'Mar') class TakeFirstItemLoader(TestItemLoader): - reduce_name = staticmethod(u" ".join) + name_red = u" ".join il = TakeFirstItemLoader() - il.add_value('name', [u'mar', u'ta']) self.assertEqual(il.get_reduced_value('name'), u'Mar Ta') - def test_loader_args(self): - def expander_func_with_args(value, loader_args): - if 'val' in loader_args: - return loader_args['val'] - return value + def test_reducer_using_classes(self): + il = TestItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_reduced_value('name'), u'Mar') + class TakeFirstItemLoader(TestItemLoader): + name_red = JoinStrings() + + il = TakeFirstItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_reduced_value('name'), u'Mar Ta') + + class TakeFirstItemLoader(TestItemLoader): + name_red = JoinStrings("
") + + il = TakeFirstItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_reduced_value('name'), u'Mar
Ta') + + def test_default_reducer(self): + il = TestItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_reduced_value('name'), u'Mar') + + class LalaItemLoader(TestItemLoader): + default_reducer = Identity() + + il = LalaItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_reduced_value('name'), [u'Mar', u'Ta']) + + def test_expander_args_on_declaration(self): class ChildItemLoader(TestItemLoader): - expand_url = tree_expander(expander_func_with_args) + url_exp = TreeExpander(expander_func_with_args, key=u'val') - il = ChildItemLoader(val=u'val') + il = ChildItemLoader() + il.add_value('url', u'text', key=u'val') + self.assertEqual(il.get_reduced_value('url'), 'val') + + def test_expander_args_on_instantiation(self): + class ChildItemLoader(TestItemLoader): + url_exp = TreeExpander(expander_func_with_args) + + il = ChildItemLoader(key=u'val') il.add_value('url', u'text') self.assertEqual(il.get_reduced_value('url'), 'val') + def test_expander_args_on_assign(self): + class ChildItemLoader(TestItemLoader): + url_exp = TreeExpander(expander_func_with_args) + il = ChildItemLoader() - il.add_value('url', u'text', val=u'val') + il.add_value('url', u'text', key=u'val') self.assertEqual(il.get_reduced_value('url'), 'val') - def test_add_value_unknown_field(self): + def test_add_value_on_unknown_field(self): il = TestItemLoader() - il.add_value('wrong_field', [u'lala', u'lolo']) + self.assertRaises(KeyError, il.add_value, 'wrong_field', [u'lala', u'lolo']) - self.assertRaises(KeyError, il.get_item) +if __name__ == "__main__": + unittest.main()