search

Crypto-toolbox / bitex

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

Question about *args and **kwargs for Nils #183

Closed firepol closed 6 years ago

firepol commented 6 years ago

Hi @nlsdfnbch,

Speaking about the dev branch...

I submitted 2 PRs and I'm working to fix the bitstamp API as well. I noticed that many methods have unnecessary unused *args and **kwargs and I removed them (see my 2 PRs). Also in some methods some parameters are set as default None if they are optional (e.g. pair=None) and sometimes they are expected from kwargs. It's a bit inconsistent and I believe that an important parameter like pair or currency should be explicitly put in the signature.

For methods with lots of parameters and eventually subject to change, I get it.

But normally APIs are not really that much subject to change, if there is a change, there is a new API version.

Also, when I see this comment # pylint: disable=unused-argument and args and kwargs are not even passed to the self.request method wonder why leaving them there? They are useless like that. Instead of a comment "unused argument" I'd rather delete them. In such cases even if a user discovers a new parameter forgotten to be implemented by bitex, he won't be able to try to use it by passing in the kwargs, as the kwargs in such cases are not passed on... so in such cases, bitex must release a hotfix anyway, so why bother?

That's why I think in many cases the methods signatures can be cleaned up quite a lot and will result in an easier use. See e.g. bitstamp.

Thoughts? I'm about cleaning up interface/bitstamp.py but if your feedback comes first, you can save me some time from doing something you'd not approve... thx

deepbrook commented 6 years ago

The interface classes, specifically the standard methods I defined, need to behave identically, no matter the kwarg / args passed.

The reason for that is quite simple: You need to be able to loop over the same method for all interfaces, without getting an unexpected argument error.

That's why for some methods, the relevant parameters like pair are seemingly optional.

I hope that answered your question.

Thanks for your contribution! I'll try to make time for a review of your PRs this weekend latest.

Cheers, Nils

firepol commented 6 years ago

Thanks for your feedback, my next question then would be, which methods are considered the standard ones? ticker, bid, ask I get... but is there a full list?

Or does this apply also to exchange specific methods or for those ones we can avoid using args and kwargs if not needed?

deepbrook commented 6 years ago

https://github.com/Crypto-toolbox/bitex/blob/dev/README.md

firepol commented 6 years ago

Ops, it was in the readme, thanks ;)