akaszynski / keepa

Python Keepa.com API
https://keepaapi.readthedocs.io/en/latest/
Apache License 2.0
241 stars 77 forks source link

Product Request: implementation of 'only-live-offers' and 'days' params and tests #96

Closed dehidehidehi closed 3 years ago

dehidehidehi commented 3 years ago

Discussing this commit: https://github.com/akaszynski/keepa/pull/96/commits/30d3f354b991fc462d6f789790c263c89fbd9675

Product Request: ✔️ 'only-live-offers' is implemented, tested, and passes test. ❌ 'days' is implemented, tested, but FAILS tests.

It would seem specifying 'days' as a parameter to the product request has no incidence of the API response... ? I don't know where the issue is. Could it possibly be from Keepa? I find it unlikely.

Keepa Request Products documentation

I hope we can discuss this in this pull request, Thank you,

spi-jbu commented 3 years ago

A few months ago, I modified version 1.1.0 to include these options.

With the following call, the 'days' option worked fine, limiting the amount of data to ~90 days:

api.query(asinList, stats=30, domain=mkt, history=False, offers=100, update=24, days=90, only_live_offers=True)

Recently, I have been using the follow which does NOT work (returning years of data) since I only need Buy Box history:

api.query(asinList, stats=30, domain=mkt, history=False, update=24, buybox=True, days=90)

It seems that if I request the 'offers' option the 'days' option works. (The 'offers' option includes the Buy Box history so I do not need to specify buybox=True)

Given the above, it may be a Keepa issue.

Hope this helps.

dehidehidehi commented 3 years ago

A few months ago, I modified version 1.1.0 to include these options.

With the following call, the 'days' option worked fine, limiting the amount of data to ~90 days:

api.query(asinList, stats=30, domain=mkt, history=False, offers=100, update=24, days=90, only_live_offers=True)

Recently, I have been using the follow which does NOT work (returning years of data) since I only need Buy Box history:

api.query(asinList, stats=30, domain=mkt, history=False, update=24, buybox=True, days=90)

It seems that if I request the 'offers' option the 'days' option works. (The 'offers' option includes the Buy Box history so I do not need to specify buybox=True)

Given the above, it may be a Keepa issue.

Hope this helps.

Thank you for this, it is helpful to know we both have issues with this. I opened a bug report on the Keepa forums.

dehidehidehi commented 3 years ago

According to Keepa support, the bug should be fixed. The days param should now limit historical data for the following "csv , buyBoxSellerIdHistory , salesRanks , offers and offers.offerCSV".

❌ However, my test still fails... ?

akaszynski commented 3 years ago

Issue with the tests failing is due to the private keys I use to run the tests. Since it might be possible to scrape the key from a malicious PR, GitHub disables foreign PRs. I'll have to figure out a workaround for this.

I'm getting a test failure at:

================================== FAILURES ===================================
___________________________ test_productquery_days ____________________________

api = <keepa.interface.Keepa object at 0x7f16455f10d0>

    def test_productquery_days(api):
        """Tests that 'days' param limits historical data to X days.
        This includes the csv, buyBoxSellerIdHistory, salesRanks, offers and offers.offerCSV fields."""

        def any_date_out_of_range(date_objects) -> bool:
            """Returns True if any date in date_objects if older than max_days, relative to now()."""
            today = datetime.datetime.now()
            return any((today - hist_date).days > max_days for hist_date in date_objects)

        max_days = 30
        request = api.query(PRODUCT_ASIN, offers=20, days=max_days)
        product = request[0]

        # Check if any Dateframes are out of date range.
        dataframes_dates = [set(df.index.tolist()) for key, df in product['data'].items() if key.startswith('df_')]
        dataframes_dates = chain.from_iterable(dataframes_dates)
        breakpoint()
>       assert not any_date_out_of_range(dataframes_dates)
E       assert not True
E        +  where True = <function test_productquery_days.<locals>.any_date_out_of_range at 0x7f1644202670>(<itertools.chain object at 0x7f163e907b20>)

tests/test_interface.py:225: AssertionError

Many of the dates are over 30 days:

2021-03-03 00:02:00
2021-02-17 16:54:00
2021-03-04 00:00:00
2021-02-25 11:02:00
2021-02-22 11:02:00
2021-03-04 11:00:00
2021-02-27 11:02:00
2021-02-26 00:02:00
2020-04-20 01:28:00
2020-04-20 01:28:00
2020-04-12 20:38:00
2020-12-25 17:00:00
2019-10-29 01:24:00
2021-03-01 19:14:00
2019-11-23 00:12:00
2021-03-01 19:14:00
dehidehidehi commented 3 years ago

Yes, the test fails although I thought I implemented the 'days' param properly. I don't know why the Keepa API response is not limiting historical data.

dehidehidehi commented 3 years ago

Did some more digging: I have confirmed Keepa does filter out historical data for 'sales_ranks', and 'offers'. However, as of now (2021-03-26), contrary to the Keepa documentation, it does not filter historical data for 'buy_box_seller_id_history' and 'offers_csv'.

Reported the bug to Keepa: https://discuss.keepa.com/t/api-product-search-days-param-only-works-along-with-offers-param/12567/4

Once bug is fixed will submit new PR with fully passing tests.