Add support for Binance Coin-M futures via binance.com-coin-futures exchange type #163

M3tz3l · 2021-04-13T22:43:05Z

PR Details

Add basic support for coin margin futures web sockets.

Description

Add new binance.com-coin-futures exchange type for wss://dstream.binance.com/ base uri
Add usage example for coin m futures websocket streams
Updated documentation
Extended unit tests
Fixed bug in unit tests

Related Issue

#134

Motivation and Context

I like the project, but I need coin margin futures 🤷‍♂️

How Has This Been Tested

Tested with the newly introduced example and by running other test examples with the changes with a Linux machine and my own Binance account. I transferred funds to my coin-m futures account to test the user data stream events.

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the
CONTRIBUTING
document.
I have added tests to cover my changes.
All new and existing tests passed.

M3tz3l · 2021-04-13T22:44:18Z

example_binance_coin_futures.py

+# Documentation: https://oliver-zehentleitner.github.io/unicorn-binance-websocket-api
+# PyPI: https://pypi.org/project/unicorn-binance-websocket-api/
+#
+# Author: Oliver Zehentleitner


Not sure about the contribution guideline regarding author here

I can add a "Thanks to M3tz3l for contributing" to the example file. Is this ok for you?

M3tz3l · 2021-04-13T22:45:03Z

unittest_binance_websocket_api.py

@@ -544,7 +544,7 @@ def test_rest_binance_com_isolated_margin_testnet(self):
        BinanceWebSocketApiRestclient(binance_websocket_api_manager)
        binance_websocket_api_manager.stop_manager_with_all_streams()

-    def test_rest_binance_com_futures_testnet(self):
+    def test_rest_binance_com_futures(self):


I think I found a bug here

thanks, the names just need to be unique, then the unittests get executed! Thanks anyway!

Unfortunately it was not unique in this case -> see line 552

i see, its double! thanks for fixing!

M3tz3l · 2021-04-13T22:45:24Z

unicorn_binance_websocket_api/unicorn_binance_websocket_api_socket.py

@@ -126,6 +126,7 @@ async def start_socket(self):
                                received_stream_data = self.unicorn_fy.binance_com_futures_websocket(received_stream_data_json)
                            elif self.exchange == "binance.com-futures-testnet":
                                received_stream_data = self.unicorn_fy.binance_com_futures_websocket(received_stream_data_json)
+                            # TODO coin margined futures (What to do about it?)


No sure what to do here

We need to update unicorn-fy too. Since i dont know whats returned from the endpoint we can start with a "try" and map coin-futures in unicorn-fy to std websocket convertation. If you want you can try, if not i can do it too, its not much work.

we will call it "binance_com_coin_futures_websocket()`

I'll try. I wanted to look into the other packages anyways.

Thanks! oliver-zehentleitner/unicorn-fy#26

How do you want this line to be fixed now? Update the comment?

just add it with a call to binance_com_coin_futures_websocket(), but maybe we should discuss #163 (comment) first

Ah ok you accept the broken call to non existing function because you'll not release it yet. I replied to #163 (comment) in #163 (comment)

yes and the unicorn-fy thing will be done very soon (from you or from me...) so i dont worry about it.

@oliver-zehentleitner done

codecov-io · 2021-04-14T08:20:01Z

Codecov Report

Merging #163 (e06f8ab) into master (d610c4d) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #163      +/-   ##
==========================================
- Coverage   91.01%   90.98%   -0.04%     
==========================================
  Files           7        7              
  Lines        1524     1530       +6     
==========================================
+ Hits         1387     1392       +5     
- Misses        137      138       +1

Impacted Files	Coverage Δ
...ocket_api/unicorn_binance_websocket_api_manager.py	`89.74% <ø> (-0.23%)`	⬇️
...socket_api/unicorn_binance_websocket_api_socket.py	`91.17% <ø> (ø)`
...et_api/unicorn_binance_websocket_api_restclient.py	`96.37% <100.00%> (+1.60%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d610c4d...e06f8ab. Read the comment docs.

oliver-zehentleitner

its fine, thanks

oliver-zehentleitner · 2021-04-14T09:27:48Z

unicorn_binance_websocket_api/unicorn_binance_websocket_api_socket.py

@@ -126,6 +126,7 @@ async def start_socket(self):
                                received_stream_data = self.unicorn_fy.binance_com_futures_websocket(received_stream_data_json)
                            elif self.exchange == "binance.com-futures-testnet":
                                received_stream_data = self.unicorn_fy.binance_com_futures_websocket(received_stream_data_json)
+                            # TODO coin margined futures (What to do about it?)


we will call it "binance_com_coin_futures_websocket()`

oliver-zehentleitner · 2021-04-14T09:30:25Z

unittest_binance_websocket_api.py

@@ -544,7 +544,7 @@ def test_rest_binance_com_isolated_margin_testnet(self):
        BinanceWebSocketApiRestclient(binance_websocket_api_manager)
        binance_websocket_api_manager.stop_manager_with_all_streams()

-    def test_rest_binance_com_futures_testnet(self):
+    def test_rest_binance_com_futures(self):


i see, its double! thanks for fixing!

oliver-zehentleitner · 2021-04-14T09:32:46Z

A question about the name:
Is "binance.com-coin-futures" the right name we should choose?
Does "binance.com-perpectual-coin-futures" makes more sense?
Can there be other types of coin futures than perpectual one?

M3tz3l · 2021-04-14T09:37:20Z

A question about the name:
Is "binance.com-coin-futures" the right name we should choose?
Does "binance.com-perpectual-coin-futures" makes more sense?
Can there be other types of coin futures than perpectual one?

I think it's fine. There are also quaterly futures. You can see them by running e.g. https://github.com/oliver-zehentleitner/unicorn-binance-websocket-api/pull/163/files#diff-59faa32510214f3e87431673745a42f9fe19b8de950f1ec79eb7452c8e5977e4R71.
I didn't include them in the example because you would have to change them periodically

M3tz3l · 2021-04-14T11:18:33Z

What's the process now? Does this PR need to wait for the unicorn-fy changes and need to be adapted afterwards or can this be done in a second step? What about this failing check? Do you merge it manually with a version bump?

oliver-zehentleitner · 2021-04-14T11:27:32Z

please finish unicorn_binance_websocket_api/unicorn_binance_websocket_api_socket.py , then i think its ready to merge.

the coverals check is ok, I care about this.

but before i create a new pypi package we have to update unicorn-fy too, or there is a call to a non existing mehtod...

oliver-zehentleitner · 2021-04-14T11:50:25Z

A question about the name:
Is "binance.com-coin-futures" the right name we should choose?
Does "binance.com-perpectual-coin-futures" makes more sense?
Can there be other types of coin futures than perpectual one?

I think it's fine. There are also quaterly futures. You can see them by running e.g. https://github.com/oliver-zehentleitner/unicorn-binance-websocket-api/pull/163/files#diff-59faa32510214f3e87431673745a42f9fe19b8de950f1ec79eb7452c8e5977e4R71.
I didn't include them in the example because you would have to change them periodically

to be honest, i dont know the differences between them. ok, lets go ahead.

oliver-zehentleitner · 2021-04-14T15:59:04Z

If you are fine with it, i would merge it!

M3tz3l · 2021-04-14T16:00:02Z

No wait one commit got lost. I should never change something in the GUI inbetween 🙈

M3tz3l · 2021-04-14T16:01:31Z

Now it should be fine

M3tz3l · 2021-04-14T16:03:12Z

Would be nice if you could provide the unicorn_fy minimum implementation. The coin-m jsons seem to be quite different. I'll try to patch it on the way when I work on it.

oliver-zehentleitner · 2021-04-14T16:01:53Z

unittest_binance_websocket_api.py

@@ -554,6 +554,11 @@ def test_rest_binance_com_futures_testnet(self):
        BinanceWebSocketApiRestclient(binance_websocket_api_manager)
        binance_websocket_api_manager.stop_manager_with_all_streams()

+    def test_rest_binance_com_coin_futures(self):
+        binance_websocket_api_manager = BinanceWebSocketApiManager(exchange="binance.com-futures")


i think you need to update to "binance.com-coin-futures"

oliver-zehentleitner · 2021-04-15T07:38:49Z

Thanks a lot for this contribution!

oliver-zehentleitner · 2021-04-15T22:28:16Z

Would be nice if you could provide the unicorn_fy minimum implementation. The coin-m jsons seem to be quite different. I'll try to patch it on the way when I work on it.

oliver-zehentleitner/unicorn-fy#26

M3tz3l added 2 commits April 14, 2021 00:14

Add coin-m futures support

d88ef51

Add coin-m futures usage example

8911e61

M3tz3l commented Apr 13, 2021

View reviewed changes

M3tz3l changed the title ~~Add coin m fut support~~ Add support for Binance Coin-M futures via binance.com-coin-futures exchange type Apr 14, 2021

Fix bug in unit tests

e06f8ab

oliver-zehentleitner approved these changes Apr 14, 2021

View reviewed changes

Transform coin-m futures websocket stream with unicorn_fy

c6e875d

oliver-zehentleitner approved these changes Apr 14, 2021

View reviewed changes

oliver-zehentleitner approved these changes Apr 15, 2021

View reviewed changes

oliver-zehentleitner merged commit 59a136e into oliver-zehentleitner:master Apr 15, 2021

Uh oh!

Add support for Binance Coin-M futures via binance.com-coin-futures exchange type #163

Add support for Binance Coin-M futures via binance.com-coin-futures exchange type #163

Uh oh!

Conversation

M3tz3l commented Apr 13, 2021

PR Details

Description

Related Issue

Motivation and Context

How Has This Been Tested

Types of changes

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Apr 14, 2021

Codecov Report

Uh oh!

oliver-zehentleitner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliver-zehentleitner commented Apr 14, 2021

Uh oh!

M3tz3l commented Apr 14, 2021

Uh oh!

M3tz3l commented Apr 14, 2021

Uh oh!

oliver-zehentleitner commented Apr 14, 2021

Uh oh!

oliver-zehentleitner commented Apr 14, 2021

Uh oh!

oliver-zehentleitner commented Apr 14, 2021

Uh oh!

M3tz3l commented Apr 14, 2021

Uh oh!

M3tz3l commented Apr 14, 2021

Uh oh!

M3tz3l commented Apr 14, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliver-zehentleitner commented Apr 15, 2021

Uh oh!