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

Refactoring crypto load #2366

Merged
merged 7 commits into from
Aug 24, 2022
Merged

Refactoring crypto load #2366

merged 7 commits into from
Aug 24, 2022

Conversation

jose-donato
Copy link
Contributor

@jose-donato jose-donato commented Aug 19, 2022

Problem:

  • coingecko currently not being used (df from coingecko is replaced by yahoofinance)
  • coingecko does not send volume
  • free coingecko tier does not allow more than 365 days and intraday data is limited
  • pycoingecko (library used to interact with pycoingecko) does not allow to insert paid key
  • "coingecko rate limits you pretty damn quickly." -> after being rate limited user cannot load coins neither enter coin-specific menus (e.g., dd, ta)
  • "Coingecko doesn't seem to be a great data provider..."
  • "why did we abandon the ability to load from a variety of sources in the Crypto menu, or the --start --end? The user is limited to only the # of days that are programmed in."
  • data from coingecko lags a bit behind

Solution:
introduce ccxt to allow users to load real data from crypto exchanges:

  • get real data from pretty much any exchange (all the following exchanges provide OHLC data ['aax', 'ascendex', 'bequant', 'bibox', 'bigone', 'binance', 'binancecoinm', 'binanceus', 'binanceusdm', 'bit2c', 'bitbank', 'bitbay', 'bitcoincom', 'bitfinex', 'bitfinex2', 'bitflyer', 'bitforex', 'bitget', 'bithumb', 'bitmart', 'bitmex', 'bitpanda', 'bitrue', 'bitso', 'bitstamp', 'bitstamp1', 'bittrex', 'bitvavo', 'bl3p', 'btcalpha', 'btcbox', 'btcmarkets', 'btctradeua', 'btcturk', 'buda', 'bw', 'bybit', 'bytetrade', 'cdax', 'cex', 'coinbaseprime', 'coinbasepro', 'coincheck', 'coinex', 'coinfalcon', 'coinmate', 'coinone', 'coinspot', 'crex24', 'cryptocom', 'currencycom', 'delta', 'deribit', 'digifinex', 'eqonex', 'equos', 'exmo', 'flowbtc', 'ftx', 'ftxus', 'gateio', 'gemini', 'hitbtc', 'hitbtc3', 'hollaex', 'huobi', 'huobijp', 'huobipro', 'idex', 'independentreserve', 'indodax', 'itbit', 'kraken', 'kucoin', 'kucoinfutures', 'kuna', 'latoken', 'latoken1', 'lbank', 'liquid', 'luno', 'mercado', 'mexc', 'ndax', 'novadax', 'oceanex', 'okcoin', 'okex', 'okex3', 'okex5', 'paymium', 'phemex', 'poloniex', 'probit', 'qtrade', 'ripio', 'stex', 'therock', 'tidebit', 'tidex', 'timex', 'upbit', 'vcc', 'wavesexchange', 'whitebit', 'xena', 'yobit', 'zaif', 'zb', 'zipmex', 'zonda'])
  • any interval - currently implemented [1- 1min, 5 - 5min, 15 - 15min, 30 - 30min, 60 - 1hr, 240 - 4hr, 1440 - 1d, 10080 - 1w, 43200 - 1M]
  • similar historical data as yf provides - only limited to when exchange listed the pair (can load 1 year with 1min of timeframe - will be slow but it's possible)

disadvantages of ccxt:

  • shitcoins not listed in major exchanges cannot be load (coingecko can be used for these situations)
  • user needs to know the pair from the exchange (we can make a command to help on this)
  • does not give an overview of the price (the price is specific to a pair in an exchange, not an overview of the market like yahoo finance or coingecko)

This PR:

  • adds ccxt as source for load
  • separates yf and cg as different sources for load - cg will have 0 as volume since they dont provide that anymore

@jose-donato jose-donato added the refactor Refactor code label Aug 19, 2022
@minhhoang1023
Copy link
Contributor

Nice one! Loaded a couple of coins and working well.

It's better to have yf as the default, to show the price overview and not specific to any exchange. Also, the volume will be more representative when the candle command is used.

  • source yf

image

  • source ccxt

image

We should also include the source / exchange in the candle chart. Otherwise, when the chart is shared or is taken out of the context, volume info is wrong (if not included the exchange name).

When loading cg, wdyt about using yf for Volume as we did previously.

@jose-donato
Copy link
Contributor Author

jose-donato commented Aug 21, 2022

Nice one! Loaded a couple of coins and working well.

It's better to have yf as the default, to show the price overview and not specific to any exchange. Also, the volume will be more representative when the candle command is used.

  • source yf
image
  • source ccxt
image

We should also include the source / exchange in the candle chart. Otherwise, when the chart is shared or is taken out of the context, volume info is wrong (if not included the exchange name).

When loading cg, wdyt about using yf for Volume as we did previously.

yea sure I will make load from yahoo finance by default @jmaslek you won after all

About cg, i don't think mixing is good. I know I suggested that before, but I'm not a fan of that approach. I say we just ditch coingecko for messari tbh

Nevermind, i agree with you. Messari just works like ccxt so it doesnt really make sense to include it as an additional load source

@minhhoang1023
Copy link
Contributor

minhhoang1023 commented Aug 23, 2022

Nice work!

Tried multiple andom shitcoins and one had this warning. Could we silence it?

image

@jose-donato jose-donato marked this pull request as ready for review August 23, 2022 19:40
@jose-donato jose-donato merged commit 4399d06 into main Aug 24, 2022
@Chavithra Chavithra deleted the load_refactor_ccxt branch September 20, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants