[Feature] fetch public trades

[TheJoeSchr]

Orderflow

Summary

This PR enables to use ccxt fetch_public_trades so freqtrade can use trades data for backtesting and trading

Quickstart:

• use --dl-trades to fetch trades for timerange
• enable using trades in config.json

"exchange": {
   ...
   "use_public_trades": true,
}

• set orderflow processing configuration in config.json:

"orderflow": {
   "scale": 0.5,
   "stacked_imbalance_range": 3,
   "imbalance_volume": 1,
   "imbalance_ratio": 300
 },

TODO

• [docs] write (new?) docs about how to access this new trades data in populate_indicators etc
  • dataframe['trades']
  • dataframe['orderflow']
  • dataframe['delta']
• [docs] how to use --dl-trades to get trade data
• [docs] which configuration options are available and how to use them
• [tests] unit tests exist, but they are using too big for github test files. so I need to shrink them and rewrite tests to still make sense

from previous feedback by @xmatthias :

• Some diffs are a bit strange at first glance (especially in the exchange class), so expect some questions once i look at it more carefully (might be the webUI doing the diffs oddly if 2 functions are next to each other and are similar enough though) ...

• and i usually don't like random formatting changes (like - an unnecessary linebreak in a function that's not even touched otherwise) => fixed with commit 137ee07

• i also assume we'll want to move some things around in the end (converter seems to become quite big now) - but i can eventually help with that.

What's new?

[biiiipy]

This is awesome! Orderflow has a lot of valuable information, can't wait to start experimenting with this!

[xmatthias]

@dijvar it's an open PR. Docs will be written before merging the PR (which the PR description also mentions).
Please don't pollute the PR discussion with pointless requests, thanks.

@TheJoeSchr you may want to look at the conflicts (don't just look at the conflicting lines though ...).
trades datahandling changed right around the time (one PR number before your's 😆 ) you submitted this PR (#9065) - so i suspect some of the code you've written is now already available - or types won't align etc.

I think the diff of #9065 will be helpful to show what changed .... i don't think much else changed ... but these changes WILL impact this PR for sure.

[TheJoeSchr]

I think the diff of #9065 will be helpful to show what changed .... i don't think much else changed ... but these changes WILL impact this PR for sure.

thanks for the headsup @xmatthias . I think the overlap is minimal, because my "trades" are about L2 data aka orderflow trades and that PR is about trades the user/bot send to the exchange to execute.

but the naming overlap is unfortunate, I'm not sure how to handle it so there won't be any more confusion

[TheJoeSchr]

Please, write a short docs to use it. Thank you very much for the work :)

Thanks for showing interest, this helps keeping my motivation up to finally write the docs. did you try the quickstart on top of this PR though? If yes, what steps didn't work?

[xmatthias]

I see a few things that i don't like and make a review very difficult
For example:

• changes that don't belong here (see explicit comment)
• docstring (and signature) changes on methods that should not have been changed, causing pointless diffs (refresh_latest_ohlcv() - for example)
• reintroduction of arrow to exchange (which has been removed in Add datetime helpers, reduce arrow usage to a minimum #8661, and was replaced with internal helpers directly for datetime)
• duplicate methods in the same class (one will overwrite the other)

Due to the above, doing a proper review is currently near impossible - a the diff is unnecessarily large (especially in the exchange file).
Please do a review yourself (for example vscode's github pull requests extension can assist you with this by showing you the diff locally, allowing you to change what you don't like), comparing the differences - and identify what you actually INTENDED to change. That's something that's VERY difficult for me to identify.

Things you think should be changed but are not directly related to this pr (for example the max_calls point) - should either be removed, or extracted into individual PR's.
this becomes increasingly important with bigger PR's - small pr's combining 2-3 things - not really a problem, usually
but in a big feature like this - very problematic - as it draws focus apart - having high potential to introduce unwanted bugs due to these "non-directly" related changes.

[TheJoeSchr]

I finally got to work on this again. I shrunk down the needed testdata from hundreds of MB to about 10 MB by using BCH as coin instead of BTC. I hope this is okay @xmatthias ? Alternatively I can still shrink it by about half the size, using 7z, but then we would need a 7z module requirement just for that...

[xmatthias]

Before doing a further review, i'd appreciate if you could look at the already open, non-solved comments.

I've noticed quite a few - most of which would be rather quick fixes (like, use an existing import instead of re-importing) ... but i've already "re-found" the same thing ... so it's pretty tedious ...

[TheJoeSchr]

Before doing a further review, i'd appreciate if you could look at the already open, non-solved comments.

I've noticed quite a few - most of which would be rather quick fixes (like, use an existing import instead of re-importing) ... but i've already "re-found" the same thing ... so it's pretty tedious ...

yes, don't do a full review yet. I'm not done with the open issues and fixes

[TheJoeSchr]

worked through open issues and resolved them

[xmatthias]

Why is writing files bad? that's something that can be tested and asserted, too.

You just need to make sure it's not in the user's directory, but in a temporary space.

You can change user_data_dir to tmp_path quite easily (i was of the opinion that this is done globally, but apparently that's not the case) by assigning that in the tests (that's also done elsewhere ...).
For these particular tests - you might wanna update it in some "local" conftest file - so it applies globally to all tests where this applies.

[TheJoeSchr]

@xmatthias don't worry about my previous message I think I figured it out with mocker.patch()

[xmatthias]

is .apply() actually necessary here?
.apply() (without additional argument) means it'll run once for each row ... but the row is not actually used, so i'm not entirely sure if it's really necessary to do this per row, or if the np.where() could work by itself - which would greatly speed up this operation.
I also can't really see a place where df.loc[is_between,...] can return more than 1 row, so that kinda reinforces me questioning if apply() is necessary for that reason either.

---

doing a quick test for this based on the test: - with a breakpoint on the sell line - testing for buy above:

np.all(buy[0] == np.where(
                        trades_grouped_df["side"].str.contains("buy"),
                        0,
                        trades_grouped_df["amount"],
                    ))

Now doing that change (use np.where() directly to assign buy/sell) also has implications below - as the "deltas per trade" loop only returns one value now - but with both changes, all tests do still pass, so i'd assume it's good, unless doing this as loop covers some odd scenario i currently can't imagine.

[TheJoeSchr]

I can't really reproduce what you changed from what you wrote, but if it is faster and the tests still pass, please feel free to just commit it. thanks

[xmatthias]

@TheJoeSchr d5361d8 is what i meant above.

please double-check results with this change ... according to tests, it seems to work fine - but tests don't always reflect all cases . . .

[TheJoeSchr]

I don't get the error in the tests, my local ones end up passing