Multiple itemadapter classes #68
Conversation
Codecov Report
@@ Coverage Diff @@
## master #68 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 318 323 +5
=========================================
+ Hits 318 323 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
itemadapter/adapter.py
Outdated
| if isinstance(obj, ItemAdapter): | ||
| return obj.asdict() | ||
| if ItemAdapter.is_item(obj): | ||
| return ItemAdapter(obj).asdict() |
There was a problem hiding this comment.
I wonder why keeping this would be needed.
There was a problem hiding this comment.
Probably a rare case, but we need to convert nested objects to dicts even if the ItemAdapter subclass itself does not handle them: 2732297
There was a problem hiding this comment.
we need to convert nested objects to dicts even if the ItemAdapter subclass itself does not handle them
Why do we need to do this? I'd expect that if a subclass e.g. removed support for dataclasses, that's on purpose, and user wants an exception to be raised. If user wants dataclasses support, it's easy to add it back explicitly.
There was a problem hiding this comment.
I think the issue could be that in the examples we don't show how to reuse the default set of adapters.
There was a problem hiding this comment.
Updated, removed the lines in question. Regarding the example, that would just be inheriting from the ItemaAdapter class and not overriding the ADAPTER_CLASSES attributes, how would you phrase that?
There was a problem hiding this comment.
@elacuesta I mean that in the examples you want to add an adapter to the existing adapter classes - this is essentially what was examples showing. But now it's a bit more complicated, because you need to use add something to the list from the base class. I'm fine with proceeding without these examples :)
|
How would one use this feature, e.g. in the scrapinghub/web-poet#136 case? I think we need to be able to set the adapter list to use in web-poet when it generates and runs tests, and the actual contents of the list need to be user-defined? @kmike what do you think? |
@wRAR This makes sense. It can also be a path to ItemAdapter class (or the class itself), not sure what's better. |
|
Are we sure it will be possible to provide some interface in web-poet to use this? I guess we could make some setting for the class list and create the subclass inside the web-poet code based on it. Asking the user to create the subclass is probably more work for the user. |
No description provided.