diff --git a/scrapy/item.py b/scrapy/item.py index d5adc1efb..4d99ea79d 100644 --- a/scrapy/item.py +++ b/scrapy/item.py @@ -25,6 +25,25 @@ class Field(dict[str, Any]): """Container of field metadata""" +def _ordered_field_names(cls: type) -> list[str]: + """Return the names of the :class:`Field` attributes of *cls* in definition + order. + + Fields declared in base classes come first, ordered from the topmost base + to the most derived class. Within each class, fields keep their definition + order. A field redefined in a subclass keeps the position of its first + definition. + """ + names: list[str] = [] + seen: set[str] = set() + for base in reversed(cls.__mro__): + for name, value in vars(base).items(): + if isinstance(value, Field) and name not in seen: + seen.add(name) + names.append(name) + return names + + class ItemMeta(ABCMeta): """Metaclass_ of :class:`Item` that handles field definitions. @@ -39,13 +58,9 @@ class ItemMeta(ABCMeta): _class = super().__new__(mcs, "x_" + class_name, new_bases, attrs) fields = getattr(_class, "fields", {}) - new_attrs = {} - for n in dir(_class): - v = getattr(_class, n) - if isinstance(v, Field): - fields[n] = v - elif n in attrs: - new_attrs[n] = attrs[n] + for n in _ordered_field_names(_class): + fields[n] = getattr(_class, n) + new_attrs = {n: v for n, v in attrs.items() if not isinstance(v, Field)} new_attrs["fields"] = fields new_attrs["_class"] = _class @@ -80,6 +95,14 @@ class Item(MutableMapping[str, Any], object_ref, metaclass=ItemMeta): #: those populated. The keys are the field names and the values are the #: :class:`Field` objects used in the :ref:`Item declaration #: `. + #: + #: Fields are kept in definition order: fields declared in base classes + #: come first, followed by fields declared in subclasses, and a field + #: redefined in a subclass keeps the position of its first definition. + #: + #: .. versionchanged:: VERSION + #: Fields are now returned in definition order rather than alphabetical + #: order. fields: dict[str, Field] def __init__(self, *args: Any, **kwargs: Any): diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index d7ea60cc8..7d751f188 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -746,7 +746,7 @@ class TestFeedExport(TestFeedExportBase): ] formats = { - "csv": b"baz,egg,foo\r\n,spam1,bar1\r\n", + "csv": b"foo,egg,baz\r\nbar1,spam1,\r\n", "json": b'[\n{"hello": "world2", "foo": "bar2"}\n]', "jsonlines": ( b'{"foo": "bar1", "egg": "spam1"}\n{"hello": "world2", "foo": "bar2"}\n' diff --git a/tests/test_item.py b/tests/test_item.py index 34b054e12..7b4c2e918 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -142,6 +142,30 @@ class TestItem: self.assertSortedEqual(list(item.keys()), ["new"]) self.assertSortedEqual(list(item.values()), ["New"]) + def test_fields_order(self): + class TestItem(Item): + name = Field() + keys = Field() + values = Field() + + assert list(TestItem.fields) == ["name", "keys", "values"] + + def test_fields_order_inheritance(self): + class ParentItem(Item): + name = Field() + keys = Field() + values = Field() + + class TestItem(ParentItem): + extra = Field() + keys = Field(serializer=str) + + # Inherited fields come first, in their definition order, followed by + # the fields newly defined in the subclass. A redefined field keeps the + # position of its first definition while taking the new metadata. + assert list(TestItem.fields) == ["name", "keys", "values", "extra"] + assert TestItem.fields["keys"] == {"serializer": str} + def test_metaclass_inheritance(self): class ParentItem(Item): name = Field()