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

Getting quotes for dates older than 1st of January 1970 #56

Open
sunreef opened this issue Nov 10, 2024 · 3 comments
Open

Getting quotes for dates older than 1st of January 1970 #56

sunreef opened this issue Nov 10, 2024 · 3 comments

Comments

@sunreef
Copy link

sunreef commented Nov 10, 2024

Description

The YResponse serialization fails when trying to access quote history older than 1st of January 1970. This is due to the timestamp of these dates becoming a negative integer.

For example, take the following code:

  let provider = yahoo::YahooConnector::new().unwrap();
  let first_trade_date = {
      let start = datetime!(2024-1-1 23:59:59.99 UTC);
      let end = datetime!(2024-2-1 23:59:59.99 UTC);
      let resp = provider
          .get_quote_history("XOM", start, end)
          .await
          .unwrap();
      let metadata = resp.metadata().unwrap();
      metadata.first_trade_date.unwrap()
  } as i64;

  // The start date is 1962-01-02 14:30:00.0 +00:00:00, so before the UNIX zero date
  let start = OffsetDateTime::from_unix_timestamp(first_trade_date).unwrap();
  let end = OffsetDateTime::now_utc();
  // Get the price history of EXXON. Dating back to the 1960s.
  // This fails with a serialization error because it's expecting timestamps in u64.
  let resp = provider
      .get_quote_history(&ticker_name, start, end)
      .await
      .unwrap();

The timestamps in the quotes are expected to be u64 so negative integers make the request fail.

Proposition

Change the timestamps type to be a i64 to allow for negative timestamps.

@cjglo
Copy link
Contributor

cjglo commented Nov 21, 2024

Happy to work on this this is wanted, the change is very easy.

Might need some discussion though as there are two ways I see we could fix this:

  1. Change the timestamp to i64, this is a breaking change and all example code will need to be changed. Also it will always require an as i64 if people are using anything that have timestamp as a u64.
    Pros: Simplest change, minimal code.
    Cons: Breaking Change that changes interface. Examples will all need to be changed. Always need an as i64 everywhere when using this crate.

  2. Duplicate the YQuoteBlock, YChart, and YResponse with the added ending BeforeEpoch and create a separate function that takes pre-1970 timestamps.
    Pros: Not a breaking change, no changes to examples or people's code using the latest version
    Cons: Gonna duplicate like 50-100 lines of code, if we want more pre-1970 stuff, there's gonna be even more duplication.

Also...

  1. Ignore this and just don't have pre-1970 historical stuff.

@cjglo
Copy link
Contributor

cjglo commented Nov 21, 2024

Don't know really how big this project is, but could leave this as a "Good first issue" label so rust beginners can do it. They should be able to do what I said above and just follow the compiler around until it is happy with all the typing.

@xemwebe
Copy link
Owner

xemwebe commented Nov 23, 2024

Happy to work on this this is wanted, the change is very easy.

1. Change the timestamp to i64, this is a breaking change and all example code will need to be changed.  Also it will always require an `as i64` if people are using anything that have timestamp as a u64.
   Pros: Simplest change, minimal code.
   Cons: Breaking Change that changes interface. Examples will all need to be changed.  Always need an `as i64` everywhere when using this crate.

I would prefer Solution 1. Code duplication is never a good idea and unfortunately, since yahoo changes the api occasionally anyway (or just stops delivering certain values in some cases, such that parameters need to be changed to become optional), strictly prohibiting breaking changes is not option.

It would be even better to parse the timestamp in a proper date time. However, because of the missing time zone information, this is difficult to be done correctly. Therefore, just switching to i64 might be the best we could do , if date before 1970 is really needed (I wasn't aware that yahoo is offering data before 1970.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants