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

[UX, Sim, Trader] DRY violations in specifying exchange #806

Closed
2 tasks
trentmc opened this issue Mar 18, 2024 · 0 comments · Fixed by #824
Closed
2 tasks

[UX, Sim, Trader] DRY violations in specifying exchange #806

trentmc opened this issue Mar 18, 2024 · 0 comments · Fixed by #824
Assignees

Comments

@trentmc
Copy link
Member

trentmc commented Mar 18, 2024

The issue

Here's a snippet from ppss.yaml:

trader_ss:
  feed: binance BTC/USDT c 5m
  sim_only:
    buy_amt: 1000 USD # buy this amount in each epoch
    fee_percent: 0.0 # simulated % fee
    init_holdings:
      - 100000 USDT
      - 2 BTC
  bot_only:
    min_buffer: 30 # in s. only trade if there's > this time left
    max_tries: 10 # max no. attempts to process a feed
    position_size: 3 # buy/sell this amount in each epoch
  exchange_only:
    timeout: 30000
    options:
      createMarketBuyOrderRequiresPrice": False
      defaultType: spot

sim_ss: # sim only
  do_plot: True
  log_dir: logs
  final_img_filebase: final_img # for name log_dir/{final_img_filebase}_{NR}.png
  test_n: 5000 # number of epochs to simulate
  tradetype: histmock # histmock | livemock | livereal
  exchange_only:
    timeout: 30000
    options:
      createMarketBuyOrderRequiresPrice": False
      defaultType: spot

The problem: exchange_only and its sub-values are in two places.

This gets reflected further downstream too.

This is a DRY violation. And, it's annoying from a UX perspective too: the user should really only have to change one place.

Candidate solutions

  1. Create a new module ExchangeSS that has exchange_only and all the info below that. Then, both Trader and Simulator see this
  2. Only have the info in TraderSS. Then, SimulatorSS also uses the info

TODO

  • Quick implementation experiment trying cand (1): will it be clean?
  • If yes to ^, build (1). Else build (2)
@trentmc trentmc changed the title [UX, Sim, Trader] DRY violation: PPSS specifies 'exchange_only' info in >1 place [UX, Sim, Trader] DRY violation: 'exchange_only' given in >1 place Mar 18, 2024
@trentmc trentmc self-assigned this Mar 18, 2024
trentmc added a commit that referenced this issue Mar 20, 2024
@trentmc trentmc changed the title [UX, Sim, Trader] DRY violation: 'exchange_only' given in >1 place [UX, Sim, Trader] DRY violations in specifying exchange Mar 20, 2024
@trentmc trentmc assigned calina-c and unassigned trentmc Apr 9, 2024
calina-c added a commit that referenced this issue Apr 9, 2024
* Fix #806: DRY violations in exchange
* Fixed tests related to the removal of ccxt_exchange.
* Use binanceus in tests.

Co-authored-by: Calina Cenan <calina@cenan.net>
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 a pull request may close this issue.

2 participants