scrapy / itemadapter

Common interface for data container classes
BSD 3-Clause "New" or "Revised" License
61 stars 13 forks source link

Make ItemAdapter.ADAPTER_CLASSES a list instead of deque #71

Closed kmike closed 1 year ago

kmike commented 1 year ago

I think it was made deque initially because of the idea that someone might want to insert to the beginning of ADAPTER_CLASSES when extending it.

However, it looks like now it only complicates the implementation and examples.

  1. I can't imagine a case when the ADAPTER_CLASSES is large enough, or when it's modified frequently enough to worry about insert performance
  2. Ideally, users shouldn't be modifying this global variable, they should be defining new ItemAdapter subclasses with custom adapter class lists, so using deque might be provoking anti-patterns
  3. It complicates extending the itemadapter for seemingly no reason: see e.g. https://github.com/scrapinghub/web-poet/pull/171/files#diff-7f611c24936cc51e81c9aecab743a1bb1d5ac3b20cb151d302e17dad893f2c54R349