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

Add time_series_intraday_extended #30

Merged
merged 6 commits into from
Dec 27, 2020

Conversation

radonnachie
Copy link
Contributor

@radonnachie radonnachie commented Dec 27, 2020

I duplicated and modified time_series_intraday to provide access to the EXTENDED function.

TODO is add to the test suite. Simple inclusion in the existing time_series tests resulted in failures.

@dm13450
Copy link
Collaborator

dm13450 commented Dec 27, 2020

@RocketRoss Do the tests pass when you run locally?
I think all the CI tests will fail when pushed because the different versions are run in parallel and overload the API.

@radonnachie
Copy link
Contributor Author

I believe so.. All of the pre-existing tests pass, except in cases where my free API limit has been reached:

fx_intraday: Test Failed at D:\Development\OpenSource\julia\dev\AlphaVantage\test\foreign_exchange_curency_test.jl:12
  Expression: length(data) === 2
   Evaluated: 1 === 2

@dm13450
Copy link
Collaborator

dm13450 commented Dec 27, 2020

Ok great, just add your test function (even if it fails) and I'll deal with the API limits

@radonnachie
Copy link
Contributor Author

Issue was that the extended function doesn't seem to take kindly to datatype=json. Fixed and added test, which passes.

@ellisvalentiner
Copy link
Owner

Tests don't pass because of too many calls to the API.

I was playing around with Base.retry but didn't have time to get it implemented.

If the tests pass for one of the jobs in the matrix then it is probably good to go.

@dm13450 dm13450 merged commit 345b906 into ellisvalentiner:master Dec 27, 2020
@dm13450
Copy link
Collaborator

dm13450 commented Dec 27, 2020

The new tests passed in some of the jobs, so I've merged.

@radonnachie
Copy link
Contributor Author

The failed tests seem to be related to Julia's version. Perhaps the new function's use of some regex to validate arguments is enabled by recent developments in Julia.

@radonnachie radonnachie deleted the intra_day_extended branch December 28, 2020 10:17
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

Successfully merging this pull request may close these issues.

None yet

3 participants