Skip to content
This repository has been archived by the owner on Jun 22, 2020. It is now read-only.

change ticker volume to base volume(ready) #68

Merged
merged 1 commit into from
Aug 16, 2017

Conversation

Swingcloud
Copy link
Contributor

No description provided.

@tmlee
Copy link
Member

tmlee commented Aug 4, 2017

For coin_exchange, the 2 volume attributes seem to return the same data
https://www.coinexchange.io/api/v1/getmarketsummaries which looks like a bug on their API

Since we are looking for base, shouldn't we opt for Volume instead of BTCVolume?

couldn't tell if it is in base or target, if its in BTC, we want to divide the price i think

@Swingcloud


EDIT:

I've checked their website,
I think we want to take Volume, because for pairs that is not against BTC, the BTCVolume will return 0. For example https://www.coinexchange.io/api/v1/getmarketsummary?market_id=112

And the Volume refers to the target. I know it is a bit confusing. Refer to @yihangho implementation https://github.com/coingecko/cryptoexchange/blob/master/lib/cryptoexchange/exchanges/coin_exchange/services/pairs.rb

Example:
https://www.coinexchange.io/api/v1/getmarketsummary?market_id=112

{"MarketID":"112","MarketAssetName":"Zeitcoin","MarketAssetCode":"ZEIT","MarketAssetID":"56","MarketAssetType":"cu","BaseCurrency":"Dogecoin","BaseCurrencyCode":"DOGE","BaseCurrencyID":"4","Active":true}

We assign our BASE as Zeitcoin
TARGET as Dogecoin

Volume returned from API is in our target currency. This is a problem because dividing the volume against the current price is not exactly correct too.

@Swingcloud
Copy link
Contributor Author

Ok! so i just change coin_exchange volume to Volume instead of BTCVolume?

@tmlee
Copy link
Member

tmlee commented Aug 4, 2017

@Swingcloud That won't be exactly correct too, since the volume is denominated in BTC.
I dont know if it is correct to divide the BTC with the base currency.
@dominicwong617 @yihangho thoughts?

@tmlee
Copy link
Member

tmlee commented Aug 4, 2017

Same issue with #58
However I do think the best solution is to divide its own price.

I've a few thoughts that I am not sure which is the best yet.

  1. Dictate and divide the target volume with the price to get volume in base
  2. If base volume is not available, then do not assign to the volume. Create a new column like volume_target. Give the user the option to use that and decide if they want to divide it.

@Swingcloud Swingcloud changed the title change ticker volume to base volume change ticker volume to base volume(WIP) Aug 7, 2017
@Swingcloud
Copy link
Contributor Author

maybe option 2 is the better way to deal with this?

@tmlee
Copy link
Member

tmlee commented Aug 14, 2017

@Swingcloud I think lets hotfix this and take care of the volume with the user by calculating it against the last price. I'll create a separate issue listing the exchange that contains this issue to revisit.

@Swingcloud Swingcloud changed the title change ticker volume to base volume(WIP) change ticker volume to base volume(ready) Aug 14, 2017
@tmlee tmlee merged commit 761fb02 into coingecko:master Aug 16, 2017
@Swingcloud Swingcloud deleted the base-volume-edit branch January 9, 2018 01:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants