betcode-org / flumine

flūmine - Betting trading framework
MIT License
173 stars 60 forks source link

Overly specific matching requirements for market.events #750

Closed petercoles closed 1 month ago

petercoles commented 5 months ago

The market.event property groups markets that share event_id and market_start_datetime values, which works fine for horse racing, but is overly prescriptive for some other sports. For example in golf Betfair does weird things with the market_start_date, so that markets that start at the same time are often given different start time and are sometimes changed them even after the market has opened. Here's a specific example from the US Masters 2024:

event ID. market ID. market type market start datetime 32261779 1.227181733 EACH_WAY 2024-04-11 12:00:00 32261779 1.212753822 WINNER 2024-04-11 12:00:00 32261779 1.212753823 TOP_5_FINISH 2024-04-11 12:00:00 32261779 1.212753825 ROUND_LEADER 2024-04-11 12:00:00 32261779 1.227323971 TOP_8_FINISH 2024-04-11 12:00:01 32261779 1.212753828 TOP_20_FINISH 2024-04-11 12:00:02 32261779 1.212753824 TOP_10_FINISH 2024-04-11 12:00:02 32261779 1.227318871 TOP_40_FINISH 2024-04-11 12:00:03 32261779 1.227334683 LIV_MASTERS_WINNER 2024-04-11 12:00:05 32261779 1.227330928 MAKE_THE_CUT 2024-04-11 12:00:05

petercoles commented 5 months ago

Tried monkey patching:

import flumine.markets.market
from collections import defaultdict

class CustomMarket(flumine.markets.market.Market):
    @property
    def event(self):
        event = defaultdict(list)
        for market in self.flumine.markets:
            if self.event_id == market.event_id:
                event[market.market_type].append(market)
        return event

flumine.markets.market.Market = CustomMarket

but a breakpoint in the replacement code is never tiggered :(

petercoles commented 5 months ago

This appears to be a way for Betfair to add additional markets and then control the order in which they appear in the tabs on their website. We clearly wouldn't want that sort of fudge polluting the Flumine code.

I suggest that a generic solution would be to make the event matching criteria configurable offering the current option (as default) and event_id only and market_start_datetime only as alternative options. Allowing the developer to set the config themselves, wehn they've worked out what strange foo Betfair is employing for the sport / region that they've writing strategies for.

The market_start_datetime option would allow elegant handling of GB greyhound racing issue where Betfair add the forecast market with a different event_id to the win & place markets.

Unless there is a better suggestion, or objection, I'll have a run at working up a pull request along these lines.

liampauling commented 5 months ago

Yes, this has been brought up before hence the filter on 1 (football) that is mad that they change the start time as oppose to adding some sort of ordering var.

I guess its a question of do we handle per event type or allow the user to configure.

Regarding the monkey patching, this is how I normally patch properties:

def _event(self):
    event = defaultdict(list)
    for market in self.flumine.markets:
        if self.event_id == market.event_id:
            event[market.market_type].append(market)
    return event

setattr(Market, "event", property(_event))
petercoles commented 5 months ago

LOL. Although running the latest version of Flumine in dev and production, my checked out version was out of date, so I wasn't aware of the filter for football. That would make the easiest solution an update to the event property in the market class:

    @property
    def event(self) -> dict:
        event = defaultdict(list)
        market_start_datetime = self.market_start_datetime
        event_type_id = self.event_type_id
        for market in self.flumine.markets.events[self.event_id]:
            if (
                market_start_datetime == market.market_start_datetime
                or event_type_id in ["1", "3"]  # <= event type added here
            ):
                event[market.market_type].append(market)
        return event

But it's only a partial solution and further incorporates the consequences of Betfair hacks into the Flumine code, which just feels icky. That said it's more user-friendly than requiring the user to realise that it's the Betfair data, and not the Flumine code, that's dodgy and discover that there's a config setting to work around it.

For the slightly different greyhound issue, your approach to monkey patching works nicely for me. It would feel like we were on a very slippery slope if we tried incorporate a solution that relies on GB-specific convention, that doesn't even stand up for historic data.

petercoles commented 5 months ago

Am I missing something, or are there currently no tests checking that the football filter works?

liampauling commented 5 months ago

Correct, looks like it was missed in this PR..

https://github.com/betcode-org/flumine/pull/657