search

yoavaviram / python-amazon-simple-product-api

A simple Python wrapper for the Amazon.com Product Advertising API ⛺
Apache License 2.0
817 stars 212 forks source link

Handle multiple ResponseGroups #60

Open jbkkd opened 9 years ago

jbkkd commented 9 years ago

The way this project currently works is by assuming there's only one response group ("Large"). The Product class is based on that response group, plus a few extras which are relevant to different ResponseGroups.

We should majorly refactor this to address different ResponseGroups and allow easy access to their fields (or Resposne Elements, as named by Amazon).

Reference for ResponseGroup can be found here, with the valid values: http://docs.aws.amazon.com/AWSECommerceService/latest/DG/ItemLookup.html

I'll be adding a few fields to the Product class meanwhile, but will try to iterate on creating a class that'll handle multiple response groups, or a class for each response group.

yoavaviram commented 8 years ago

Sounds interesting. Have you done any work on this?

jbkkd commented 8 years ago

I've done some work but it's internal only currently, I hope I'll be able to open source it soon.

yoavaviram commented 8 years ago

Please share as soon as you feel it's ready.

JasonCrowe commented 7 years ago

I would also be interested in this. I don't understand the xml well enough to make it happen, but I would be willing to help where I can.

ThomasProctor commented 6 years ago

Ooops. Didn't see this and opened another feature request #133.

ThomasProctor commented 6 years ago

Should these features be added as new methods (ex api.lookup_alternate_versions(...)) or using the ResponseGroup keyword argument?

ThomasProctor commented 6 years ago

It looks like the most recent version handles the "Images" response group by adding the property "images", which can be used by using the ResponseGroup keyword argument.

ThomasProctor commented 6 years ago

I went ahead and submitted a pull request for the features that I wanted to use. It's modeled on how we dealt with the "Images" response group.

Some of the other response groups might not be such low hanging fruit. The "Variations" response group, for instance, has a much more complex tree structure, which might be inconsistent across different searches. Maybe each variation will adapt well to the AmazonProduct class, and a variations property can return a list of AmazonProducts?

As @jbkkd mentioned, it might be worthwhile to just refactor the whole code base (Use different methods for each response group, and have each response have a different class?), but it didn't seem like that was going to happen any time soon.

yoavaviram commented 6 years ago

@jbkkd, would you be interested in attempting the re-factor / becoming a core committer?

ThomasProctor commented 6 years ago

It might be a good idea to focus on documentation of how this works in the short term. If we mention somewhere which response groups currently work and which won't, that would go a long way towards improving the UI. Maybe the refactor won't be 100% necessary if we do that well. If we could warn or raise an error if a user passes an unsupported response group, that would probably help a lot too.

Right now, "BrowseNodeInfo", "Large", "Images", and "AlternateVersions" are supported explicitly. I imagine we could add support for "Small" and "Medium" by just writing some tests for them?