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

🥅 Handle data limits from each Investing.com request #27

Merged
merged 26 commits into from
Oct 14, 2022
Merged

Conversation

alvarobartt
Copy link
Owner

@alvarobartt alvarobartt commented Oct 9, 2022

✨ Features

  • Add conversion for all dates with times to UTC to avoid timezone issues
  • Set to_date param's default value to the current date
  • Improve request_to_investing error handling
  • Add interval handling for: "D", "W", "M", 1 min, 5 mins, 15 mins, 30 mins, 60 mins and 300 mins

🐛 Bug Fixes

  • Handle data limits per each request to Investing.com (when interval="D" the maximum amount of data to retrieve is 19 years of data, just like in investpy)
  • Fix conversion of %m/%d/%Y dates to UTC, as for the UTC+ timezones, it was reducing the date by the hours skew, which was resulting in wrong historical data retrieval results
  • Fix interval calculation in calculate_date_intervals as while loop was calculating to_datetime minus from_datetime difference, when what had to be calculated for the loop to continue for more than one iteration was max_to_datetime minus to_datetime

🔮 TODOs

  • Handle data limits for all available intervals
    • Daily -> Limit is around 19 years
    • Weekly -> No data limits
    • Monthly -> No data limits
    • Minutes (1, 5, 15, 30, 45, 60, 120, 240, 300)
  • Is https://tvc4.investing.com also limited when retrieving intraday data? -> Yes, same limit as interval="D" on 5000 results
  • In order to provide useful intraday data, we should also provide the input format %H:%M %m/%d/%Y for both from_date and to_date -> At the end we decided to use %m/%d/%Y %H:%M instead
  • Add/Explain limitations in documentation

📌 Additional Information

It seems that both https://tvc4.investing.com and https://tvc6.investing.com APIs have a limitation of 5000 results, which for interval="D" seems something easy to calculate, while for intra-day intervals is a little bit harder as the amount of minutes/hours in a trading day depends on the exchange, but we'll be calculating an estimation over NASDAQ or NYSE assets, as those are the most active ones, so as to overestimate the intervals and make sure we don't lose data.

🔗 Linked Issue/s

#24 #28 #31

🧪 Tests

  • Did you implement unit tests if required?

If the above checkbox is checked, describe how you unit-tested it.

In order to test historical_data with wider date ranges, we've included a new unit test named test_historical_wide_range and two new values for both from_date and to_date named from_date_wide_range and to_date_wide_range, so as to make sure that we can retrieve daily (interval="D") data of more than 19 years.

@alvarobartt alvarobartt added the bug Something isn't working label Oct 9, 2022
@alvarobartt alvarobartt self-assigned this Oct 9, 2022
@alvarobartt alvarobartt linked an issue Oct 9, 2022 that may be closed by this pull request
As done in `investpy`, it seems that Investing.com APIs are limited to a certain number of data points, in this case the limit seems to be in 19 years (for "D" interval only)
This function has been created so as to support the calculation of datetimes for all the available intervals, as https://tvc4.investing.com is limited, otherwise results will be shortened
Handle both supported date formats, improve timezone conversion just for "D", "W", and "M" intervals (fixes problem with today's data #28), and add some useful comments.
@alvarobartt alvarobartt linked an issue Oct 11, 2022 that may be closed by this pull request
@alvarobartt alvarobartt linked an issue Oct 11, 2022 that may be closed by this pull request
@InnovArul
Copy link

InnovArul commented Oct 11, 2022

Hi, I was testing this branch. I wanted to mention a usecase to know how to handle it.
Basically I would like to retrieve daily timeframe data for a particular stock from the beginning when it was listed (it can be any stock, I just want to specify the intrumentid, but from_date='01/01/1950' just to be sure to get from the beginning).

This call fails with error ConnectionError: Request to Investing.com API failed with error message: no_data..
This usecase was working with investpy where it gave data automatically from the actual start date corresponding to that stock.
Can we handle this in investiny as well? basically we may need to skip the exception with message no_data and only consider the time intervals which give data?

if endpoint == "history" and d["s"] != "ok":
raise ConnectionError(
f"Request to Investing.com API failed with error message: {d['s']}."
if "nextTime" not in d
else (
f"Request to Investing.com API failed with error message: {d['s']}, the"
" market was probably closed in the introduced dates, try again with"
f" `from_date={datetime.fromtimestamp(d['nextTime'], tz=timezone.utc).strftime(Config.date_format)}`."
)
)

PS.: I am sorry for intruding into this branch:)

@alvarobartt
Copy link
Owner Author

Hi, I was testing this branch. I wanted to mention a usecase to know how to handle it. Basically I would like to retrieve daily timeframe data for a particular stock from the beginning when it was listed (it can be any stock, I just want to specify the intrumentid, but from_date='01/01/1950' just to be sure to get from the beginning).

This call fails with error ConnectionError: Request to Investing.com API failed with error message: no_data.. This usecase was working with investpy where it gave data automatically from the actual start date corresponding to that stock. Can we handle this in investiny as well? basically we may need to skip the exception with message no_data and only consider the time intervals which give data?

if endpoint == "history" and d["s"] != "ok":
raise ConnectionError(
f"Request to Investing.com API failed with error message: {d['s']}."
if "nextTime" not in d
else (
f"Request to Investing.com API failed with error message: {d['s']}, the"
" market was probably closed in the introduced dates, try again with"
f" `from_date={datetime.fromtimestamp(d['nextTime'], tz=timezone.utc).strftime(Config.date_format)}`."
)
)

PS.: I am sorry for intruding into this branch:)

Sure, makes sense, but I'll need to figure out how!

@InnovArul
Copy link

What if we return an empty dict when there is no_data?
I see it's working for my use case with the patch ae4ff46.

@alvarobartt
Copy link
Owner Author

What if we return an empty dict when there is no_data?

I see it's working for my use case with the patch ae4ff46.

Well that will work for sure, but ideally we should avoid sending too many requests to Investing.con so I'll try to do it during the interval calculation, and then also to return an empty dict in that scenario!

@InnovArul
Copy link

Yes, if there is a way to eliminate empty data time intervals and find the actual from_date automatically, that will be ideal!

Type-hints are already defined in the function args, so there's no need to add that information in the docstrings too.
If `from_date` is provided and `to_date` is not, then `to_date` defaults to current date in UTC format.
Also intervals: 45, 120, and 240, have been removed as those have been included recently and most of the assets don't have those among their `intraday_multipliers`
Used via `znck.grammarly` VSCode extension.
@alvarobartt alvarobartt marked this pull request as ready for review October 14, 2022 12:00
@alvarobartt alvarobartt merged commit 4558cda into main Oct 14, 2022
@alvarobartt alvarobartt deleted the data-limit branch October 14, 2022 12:01
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

Successfully merging this pull request may close these issues.

Getting incorrect Date and Prices problem with today's data 5000 rows limit
2 participants