-
Notifications
You must be signed in to change notification settings - Fork 49
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: struct and handling of klines from binance #38
Conversation
lib/binance.ex
Outdated
def get_klines(symbol, interval, limit \\ 500) when is_binary(symbol) do | ||
case HTTPClient.get_binance("/api/v3/klines?symbol=#{symbol}&interval=#{interval}&limit=#{limit}") do | ||
{:ok, data} -> | ||
#IO.inspect data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the commented out code?
:ignore | ||
] | ||
|
||
def new(list) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is not needed because we're using ExConstructor
Just add use ExConstructor
at the bottom. Checkout
binance.ex/lib/binance/exchange_info.ex
Lines 6 to 14 in 7a9ee01
defstruct [ | |
:timezone, | |
:server_time, | |
:rate_limits, | |
:exchange_filters, | |
:symbols | |
] | |
use ExConstructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm. I tried to do it, but Binance actual returns the kline info as a List, not a Map. Take a peek here: https://binance-docs.github.io/apidocs/spot/en/#kline-candlestick-data - maybe ExConstructor is smart enough to figure out the struct load, but I'm not sure how to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right. Don't know why I expected a struct. Hrrm, using positions as mapping isn't ideal but there's no good alternative I guess
Thanks for the pull request! I added some comments Also could you rebase the latest master? I expanded the CI to run on branches |
LGTM! Thanks for the contribution |
Released in https://github.com/dvcrn/binance.ex/tree/v0.8.0 |
Basic handling of https://binance-docs.github.io/apidocs/spot/en/#kline-candlestick-data . Tried to follow the basic struct pattern and added a test as well.