Crypto-toolbox / bitex

Crypto-currency Exchange API Framework
MIT License
484 stars 136 forks source link

Fix several bitstamp API issues #184

Closed firepol closed 6 years ago

firepol commented 6 years ago

Fix calls to endpoints beginning with "api/". Allow to get all user_transactions at once (pair is optional!). Allow to get deposit address and to withdraw BCH. Add missing withdrawal_requests method. Remove inexistent withdrawal_request method (withdraw exists already). Fix transfer_sub_to_main and transfer_main_to_sub endpoints. Various DRY-ups, clean-ups, code style improvements.

firepol commented 6 years ago

@nlsdfnbch : based on your feedback on my other PRs I kept the args/kwargs in the standardized methods, but removed it when not needed, in the exchange specific methods.

Also, the withdraw/deposit methods were not supporting all cryptos, e.g. BCH was missing. I initially did some logic to check the valid ones, but based on your feedback on cryptopia it seems better not to bother, if the user tries to withdraw an invalid currency, the API will fail and an exception will be thrown.

I noticed that bitcoin and ripple used a different notation (currency entire name, not symbol), but the new ones use the symbol, so like this if bitstamp adds new coins, it should work also for the future ones, unless they become "creative" and change naming convention and become inconsistent.

firepol commented 6 years ago

@nlsdfnbch I think this PR is ready for merging, I fixed it based on your feedback. Anything else blocking the merge?

deepbrook commented 6 years ago

There are a few minor things, but I haven't had the time to write them up. I'll do so later today

deepbrook commented 6 years ago

Actually, python is smart about its parameters - if you pass pair as a positional argument, but no positional argument is defined in the function signature, it'll automatically assign it to the first explicitly defined kwarg. So, despite having it defined as a keyword argument, you can still call it like so:

b.hourly_ticker('btcusd')

On 8 May 2018 19:42:35 CEST, firepol notifications@github.com wrote:

firepol commented on this pull request.

         except AttributeError:
  • pair = kwargs['pair']
  • return self.request('balance/%s/' % pair, authenticate=True, params=kwargs)
  • return self.request('balance/', authenticate=True, params=kwargs)
  • pair = pair

I found a better way: pass

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/Crypto-toolbox/bitex/pull/184#discussion_r186810257

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

firepol commented 6 years ago

Hi,

I tried it using this mwethod with kwargs:

def hourly_ticker(self, **kwargs): """Return the hourly ticker for the given pair.""" pair = kwargs.get('pair') or '' return self.request('ticker_hour/' + pair)

In python 3.6 this doesn't work for me:

import bitex bitstamp = bitex.Bitstamp() hourly = bitstamp.hourly_ticker('xrpusd').json() print(hourly)

I get this error:

hourly = bitstamp.hourly_ticker('xrpusd').json()

TypeError: hourly_ticker() takes 1 positional argument but 2 were given

Also, the bitstamp v2 API actually requires the pair, so I really don't see why not just putting it in the method... Bitstamp v1 api seems it was done when they were actually offering only btcusd (thats' why lots of old methods don't need a parameter and actually offer btcusd as default), super old now, nobody should use it...

On Tue, May 8, 2018 at 8:00 PM, Nils Diefenbach notifications@github.com wrote:

Actually, python is smart about its parameters - if you pass pair as a positional argument, but no positional argument is defined in the function signature, it'll automatically assign it to the first explicitly defined kwarg. So, despite having it defined as a keyword argument, you can still call it like so:

b.hourly_ticker('btcusd')

On 8 May 2018 19:42:35 CEST, firepol notifications@github.com wrote:

firepol commented on this pull request.

except AttributeError:

  • pair = kwargs['pair']
  • return self.request('balance/%s/' % pair, authenticate=True, params=kwargs)
  • return self.request('balance/', authenticate=True, params=kwargs)
  • pair = pair

I found a better way: pass

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/Crypto-toolbox/bitex/pull/184#discussion_r186810257

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Crypto-toolbox/bitex/pull/184#issuecomment-387489654, or mute the thread https://github.com/notifications/unsubscribe-auth/ABn7PoTzK9NZ19lE7hz7r0LgSxHSJRgtks5twd1IgaJpZM4TgsLT .

deepbrook commented 6 years ago

You misunderstood me. Try this:

def foo(bar=None, **kwargs):
    print(bar)

foo("cheese balls")

On 8 May 2018 22:00:34 CEST, firepol notifications@github.com wrote:

Hi,

I tried it using this mwethod with kwargs:

def hourly_ticker(self, **kwargs): """Return the hourly ticker for the given pair.""" pair = kwargs.get('pair') or '' return self.request('ticker_hour/' + pair)

In python 3.6 this doesn't work for me:

import bitex bitstamp = bitex.Bitstamp() hourly = bitstamp.hourly_ticker('xrpusd').json() print(hourly)

I get this error:

hourly = bitstamp.hourly_ticker('xrpusd').json() TypeError: hourly_ticker() takes 1 positional argument but 2 were given

Also, the bitstamp v2 API actually requires the pair, so I really don't see why not just putting it in the method... Bitstamp v1 api seems it was done when they were actually offering only btcusd (thats' why lots of old methods don't need a parameter and actually offer btcusd as default), super old now, nobody should use it...

On Tue, May 8, 2018 at 8:00 PM, Nils Diefenbach notifications@github.com wrote:

Actually, python is smart about its parameters - if you pass pair as a positional argument, but no positional argument is defined in the function signature, it'll automatically assign it to the first explicitly defined kwarg. So, despite having it defined as a keyword argument, you can still call it like so:

b.hourly_ticker('btcusd')

On 8 May 2018 19:42:35 CEST, firepol notifications@github.com wrote:

firepol commented on this pull request.

except AttributeError:

  • pair = kwargs['pair']
  • return self.request('balance/%s/' % pair, authenticate=True, params=kwargs)
  • return self.request('balance/', authenticate=True, params=kwargs)
  • pair = pair

I found a better way: pass

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub:

https://github.com/Crypto-toolbox/bitex/pull/184#discussion_r186810257

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub

https://github.com/Crypto-toolbox/bitex/pull/184#issuecomment-387489654, or mute the thread

https://github.com/notifications/unsubscribe-auth/ABn7PoTzK9NZ19lE7hz7r0LgSxHSJRgtks5twd1IgaJpZM4TgsLT .

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/Crypto-toolbox/bitex/pull/184#issuecomment-387524298

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

firepol commented 6 years ago

Aaaah ok I get it now ;)

But I still don't understand what would be the code change to do here.

Let's have a look at the old method:

@check_and_format_pair
def hourly_ticker(self, pair, **kwargs):
    """Return the hourly ticker for the given pair."""
    if pair:
        return self.request('ticker_hour/%s/' % pair, params=kwargs)
    return self.request('api/ticker_hour/')

New method:

@check_and_format_pair
def hourly_ticker(self, pair):
    """Return the hourly ticker for the given pair."""
    return self.request('ticker_hour/' + pair)

I removed kwargs, because this is not a standardized method and also because no additional parameters are needed here.

The old method supports a call without "pair", but it's wrong: the method signature should have been pair=None. Anyway, even with fixed signature: If no pair is given, by default it'll return the hourly ticker for btcusd using the old API call. Not really needed on my opinion (because it makes more sense to pass a pair, not just to expect a default, even if the bitstamp api allows that), thus I cleaned up the code like that.

I removed kwargs also in: eur_usd_conversion_rate(self)

Hope it works for you?

On Tue, May 8, 2018 at 10:50 PM, Nils Diefenbach notifications@github.com wrote:

You misunderstood me. Try this:

def foo(bar=None, **kwargs):
print(bar)

foo("cheese balls")

On 8 May 2018 22:00:34 CEST, firepol notifications@github.com wrote:

Hi,

I tried it using this mwethod with kwargs:

def hourly_ticker(self, **kwargs): """Return the hourly ticker for the given pair.""" pair = kwargs.get('pair') or '' return self.request('ticker_hour/' + pair)

In python 3.6 this doesn't work for me:

import bitex bitstamp = bitex.Bitstamp() hourly = bitstamp.hourly_ticker('xrpusd').json() print(hourly)

I get this error:

hourly = bitstamp.hourly_ticker('xrpusd').json() TypeError: hourly_ticker() takes 1 positional argument but 2 were given

Also, the bitstamp v2 API actually requires the pair, so I really don't see why not just putting it in the method... Bitstamp v1 api seems it was done when they were actually offering only btcusd (thats' why lots of old methods don't need a parameter and actually offer btcusd as default), super old now, nobody should use it...

On Tue, May 8, 2018 at 8:00 PM, Nils Diefenbach notifications@github.com wrote:

Actually, python is smart about its parameters - if you pass pair as a positional argument, but no positional argument is defined in the function signature, it'll automatically assign it to the first explicitly defined kwarg. So, despite having it defined as a keyword argument, you can still call it like so:

b.hourly_ticker('btcusd')

On 8 May 2018 19:42:35 CEST, firepol notifications@github.com wrote:

firepol commented on this pull request.

except AttributeError:

  • pair = kwargs['pair']
  • return self.request('balance/%s/' % pair, authenticate=True, params=kwargs)
  • return self.request('balance/', authenticate=True, params=kwargs)
  • pair = pair

I found a better way: pass

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub:

https://github.com/Crypto-toolbox/bitex/pull/184#discussion_r186810257

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub

<https://github.com/Crypto-toolbox/bitex/pull/184#issuecomment-387489654 , or mute the thread

https://github.com/notifications/unsubscribe-auth/ ABn7PoTzK9NZ19lE7hz7r0LgSxHSJRgtks5twd1IgaJpZM4TgsLT .

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/Crypto-toolbox/bitex/pull/184#issuecomment-387524298

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Crypto-toolbox/bitex/pull/184#issuecomment-387538037, or mute the thread https://github.com/notifications/unsubscribe-auth/ABn7PloeINafDw8zWvNpPrH7SwHR4FeFks5twgT-gaJpZM4TgsLT .