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

adds proper sell rounding to avoid LOT errors #79

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hExPY
Copy link
Contributor

@hExPY hExPY commented May 14, 2021

Adds proper coin rounding in order to fix LOT error; reformat file; adds handling when someone uses a version without trailing loss/profit; adds .gitignore; updates SDK to recent version

… using an editor; adds handling when someone uses a version without trailing loss/profit; adds .gitignore; updates SDK to recent version
Copy link

@zkrige zkrige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better than my fixes. nice one


sell_amount = coins_bought[coin]['volume']
tick_size = float(next(
filter(lambda f: f['filterType'] == 'LOT_SIZE', client.get_symbol_info(coin)['filters'])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you can cache lot_size into coins_bought.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes perfect sense. I'll implement it.

@hExPY hExPY marked this pull request as draft May 14, 2021 15:40
@hExPY hExPY marked this pull request as ready for review May 14, 2021 16:57
@hExPY
Copy link
Contributor Author

hExPY commented May 14, 2021

Been running this version for some hours now. Not even one error occured. :)

@mindflvx
Copy link

Been testing for a while here too, and so far no errors. Thanks!

@Davidobot
Copy link
Contributor

@CyberPunkMetalHead this one seems important to merge

@getsec
Copy link
Collaborator

getsec commented May 19, 2021

@hExPY Can you please resolve conflicts?

@hExPY
Copy link
Contributor Author

hExPY commented May 24, 2021

Was on vacation. Now I merged. :)

Copy link
Contributor

@Markkop Markkop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an official dev for this project, but left some suggestions. Feel free give your opinion on them

Comment on lines 365 to 373
try:
TP = float(coins_bought[coin]['bought_at']) + (
float(coins_bought[coin]['bought_at']) * coins_bought[coin]['take_profit']) / 100
SL = float(coins_bought[coin]['bought_at']) + (
float(coins_bought[coin]['bought_at']) * coins_bought[coin]['stop_loss']) / 100
# When an older version of the script is being executed
except KeyError:
TP = float(coins_bought[coin]['bought_at']) + (float(coins_bought[coin]['bought_at'])) / 100
SL = float(coins_bought[coin]['bought_at']) + (float(coins_bought[coin]['bought_at'])) / 100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be a good idea to put coins_bought[coin]['bought_at'] into a variable to avoid repeating it too much?
Code might be cleaner

Comment on lines +397 to +399
tick_size = float(next(
filter(lambda f: f['filterType'] == 'LOT_SIZE', client.get_symbol_info(coin)['filters'])
)['stepSize'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line seems to be too long. Is there any way to make it more readable? (I really don't know)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested this yet, will it leave crypto dust for people using BNB for fees?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it will leave crypto dust which can be changed into BNB afterward.

adds code review changes
Comment on lines +367 to +368
TP = BuyPrice + (BuyPrice) * coins_bought[coin]['take_profit']) / 100
SL = BuyPrice + (BuyPrice) * coins_bought[coin]['stop_loss']) / 100
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code doesn't run. Remove the extraneous parenthesis:

Suggested change
TP = BuyPrice + (BuyPrice) * coins_bought[coin]['take_profit']) / 100
SL = BuyPrice + (BuyPrice) * coins_bought[coin]['stop_loss']) / 100
TP = BuyPrice + (BuyPrice) * coins_bought[coin]['take_profit'] / 100
SL = BuyPrice + (BuyPrice) * coins_bought[coin]['stop_loss'] / 100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants