Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility of period overflow #200

Open
asehmi opened this issue Sep 24, 2021 · 2 comments
Open

Possibility of period overflow #200

asehmi opened this issue Sep 24, 2021 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@asehmi
Copy link

asehmi commented Sep 24, 2021

Hi - this library is very interesting and perusing and running the code I noticed that there's a possibility of the download period calculation going out of bounds, which throws an error if it steps into the future (i.e., in general there is no guarantee that ((end_time - start_time) % step_size) == 0, except when step_size is set to one day and the interval is 1d ). Here's your code:

    while start_time <= end_time:
        period = start_time + step_size
        candlestick = exchange.get_candles(
            ticker=ticker,
            time_interval=interval,
            start_time=start_time,
            end_time=period,
        )
        candle_data.extend(candlestick)
        write_to_console(
            ticker,
            interval,
            candlestick,
            live,
            setup_table(),
        )
        live.refresh()
        start_time = period
        time.sleep(_RATE_LIMIT)

... whereas I suggest it should be (period has been renamed to period_end):

    while start_time < end_time:
        period_end = start_time + step_size
        if period_end > end_time:
            period_end = end_time
        candlestick = exchange.get_candles(
            ticker=ticker,
            time_interval=interval,
            start_time=start_time,
            end_time=period_end,
        )
        candle_data.extend(candlestick)
        write_to_console(
            ticker,
            interval,
            candlestick,
            live,
            setup_table(),
        )
        live.refresh()
        start_time = period_end
        time.sleep(_RATE_LIMIT)

(step_size should probably also be adjusted based on the interval, though I haven't shown that.)

Hope that makes sense.

Thanks,
Arvindra

@asehmi asehmi added the bug Something isn't working label Sep 24, 2021
@create-issue-branch
Copy link

Branch issue-200-Possibility_of_period_overflow created!

@Corfucinas
Copy link
Owner

Hi @asehmi , yes, you're correct in your observations.

I'll take a look and update it when I have a bit more time on my hands. Thanks for opening this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants